bloodhound-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Olemis Lang <ole...@gmail.com>
Subject Re: ProductEnvironment constructor
Date Fri, 27 Sep 2013 16:02:43 GMT
On 9/27/13, Anze Staric <anze.staric@gmail.com> wrote:
> Is there anything wrong with replacing
>
>          if not isinstance(env, trac.env.Environment):
>               raise TypeError("Initializer must be called with " ...
>
> with
>
>         env = env.parent or env
>
> in ProductEnvironment constructor?

I'd rather prefer to continue raising that error because ...

> The assertion is there to prevent
> nesting of ProductEnvironments, but this is still achieved with the
> bottom version.

... that's not the only reason . By reviewing meta-class code you'll
notice that instances of ProductEnvironment are cached considering
initializer input parameters i.e. global env + prefix . That been said
this is how I see it

  - If product envs will be allowed as first arg with no further changes then
    * Key combinations that should point at a single instance will grow ...
      exponentially ? O(n^2) ? ... in any case that's pretty bad
    * ... thus leading to situations in which multiple copies of the same
      product env will be instantiated
    * LRU caching will be more problematic than helpful

That's why I wrote separate class function for this [1]_ (btw ,
highlighting in repository browser will not work until an eventual
upgrade will be performed, nevertheless pay attention to the lines
supplied in marks URL parameter ...)

> Extracting global env when product env is passed would
> simplify a lot of places in code where a check is needed before
> instantiating ProductEnvironment. It would also pass all the failing
> wiki syntax tests.
>

The problem with this approach has been described above , which is a
major problem ; even if your proposal is very interesting in terms of
uniformity .

.. [1] http://goo.gl/KN8j6S

-- 
Regards,

Olemis - @olemislc

Mime
View raw message