flex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Erik de Bruin <e...@ixsoftware.nl>
Subject Re: [FalconJx] Refactor Notes of the last couple commits
Date Fri, 01 Mar 2013 13:01:33 GMT

> - '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.



Ix Multimedia Software

Jan Luykenstraat 27
3521 VB Utrecht

T. 06-51952295
I. www.ixsoftware.nl

View raw message