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
