> > What's the difference in usage between 'eval' and 'exec' in the
> > loader classes? Are either of these overridden ever?
>
> This is documented in the javadoc.
>
> Essentially eval doesn't return a result and exec does (or visa versa, I
> can't remember off the top of my head right now). Thus, Modules have the
> option to return a value (ConcreteElement) or not.
Ok. Starting to understand it.
>
> > 1. Rename 'eval' and 'exec' to more descriptive names. If they're not
> > used independently, merge them into a single method.
>
> See my last message about making a lot of changes that will affect a lot
of
> code. ;-)
I certainly understand the importance of that issue. On the other hand
I've found that good rewriting and a good deprecation strategy allows the
code to move forward. An equally important issue is code maintainability
and readability. Just quickly scanning through the code was not enough for
me to determine what 'eval' and 'exec' did. Not to mention the fact that
the 'build' method for layouts doesn't return a value and the 'build'
method for screens does which is a bit inconsistent.
A good deprecation strategy is something like: leave deprecated support in
the next major version, phase it out in the following major version. This
gives users a chance to migrate... For instance, leave 'build' support in
for 2.0 but plan on getting rid of it (and telling users so) in 3.0.
By the way, instead of 'exec' and 'eval', method names like
'addScreenElementToPage' and 'buildScreenElement' seem nicer.
I hope you consider my messages as constructive. Turbine seems to be a
nicely put together system and I don't mean to be overly critical and I am
certainly aware of the issues of migrating libraries. My day job includes
an object-oriented script language with 1600 functions that I have to
migrate constantly -- so I'm well aware of the issues that go along with
an external user base and I appreciate the fact that someone who is aware
of these issues is an active developer on this project!
The thing that appeals to me about open source projects is the ability to
contribute in a constructive way to make products that I'm interested in
work a LOT better! My comments are directed towards that end so please
don't think that I don't appreciate the work and design that has already
gone into Turbine. It's a cool system.
>
> > 2. Make PageLoader and LayoutLoader follow the model of the other
classes.
>
> they already do. What is different?
PageLoader and LayoutLoader do not have 'eval' methods. I'm not sure
they're needed but I don't see why they should be any different from the
other ones.
>
> > 3. Perhaps move all of this into a parent class which implements the
> > duplicated code in one place.
>
> Can't do that in order to cleanly maintain the interface stuff...
I'll study the code more. I don't see why but I also haven't looked at
every case in the code. Since all except for PageLoader and LayoutLoader
have a 'eval' method that is invoked by 'exec' in the default
implementation, I was suggesting moving 'exec' into GenericLoader and
making it invoke 'eval' in the default case... Not sure if this is the
exact right way to go so I'll study more... Any comments?
>
> > 4. Instead of invoking 'build' directly, invoke another method like
> > 'do_build' which invokes 'build'. I try to make abstract methods only
> > invoked from INSIDE a particular class. This allows the class to
> > change calling context of the abstract method easily.
>
> It seems as though you thought about this whole bit more in your patch
that
> you sent after this message so I'm not going to comment on this again
> because I already have. ;-)
Yes, again, thanks for the feedback. It's well appreciated.
>
> > Again I apologize for the barrage of questions... Just trying to get
> > to the point where I can make useful changes to the code.
>
> No worries, just be VERY careful about making suggestions that might
affect
> large bodies of code. There are a lot of people using Turbine as it is.
It
> hasn't been released yet because I knew that some of these changes
needed to
> be made...but we still have to be careful of these things.
------------------------------------------------------------
To subscribe: [EMAIL PROTECTED]
To unsubscribe: [EMAIL PROTECTED]
Problems?: [EMAIL PROTECTED]
