Mike,
- 'common' Although I agree with you making things common, I don't agree
with having sub packages, it seems redundant and confusing, if a sub package
warrants in common, it should have a real package in compiler.
Might be non-native speaker choosing names... The code I put in
'common' is code that 'as' and 'mxml' have in common. In that sense,
for symmetry's sake, I thought it should also have the same sub
package structure as both 'as' and 'mxml'. Maybe 'shared' or 'general'
might have been better names for the package?
- On the note of this change, if we could have talked about this first you
would have heard me first say that maybe we should move 'compiler.as' and
'compiler.js' into a 'compiler.codegen'
...
The same change could be applied to 'driver'. This was a mistake on my part
when I was originally laying out the first impl of the packages.
I see two "mistakes" (your original layout and me blindly
copying/extending it) combined creating one humongous refactor when
things quiet down a bit ;-)
- Tests, I don't understand why addLibraries() and such in ITestBase are
public API. They will never be called outside of the test. In java you would
make them abstract and the TestBase class abstract and that creates the
subclass contract implicitly.
At least one of them has an implementation in TestBase, and the others
may well have one in the future... I'm not that familiar with Java -
and AS has no 'abstract' - but wouldn't having an implementation
prohibit the declaration from being abstract?
- Also, passing the List as a parameter of those 3 methods encapsulates the
actual field, then if you are just overridding them in a sub class, you are
not trying to figure out what field goes where, its just a template method
that you add entries to the list passed.
That was your original implementation, but again my lack of
familiarity with Java is probably causing me some problems here. I
figured that calling 'super' with the list as argument might cause
problems. In hindsight, it might have been a different aspect of my
implementation that was causing the problem, prompting the change and
loosing the advantage of the original approach (would this still work
when using ITestbase?)
I hope you can think about these issues, maybe we can change them down the
road. Right now there are no reverting necessary.
Excellent!
EdB
--
Ix Multimedia Software
Jan Luykenstraat 27
3521 VB Utrecht
T. 06-51952295
I. www.ixsoftware.nl
