Create an abstraction layer over as3commons-reflect

[jonnyreeves]

Currently Vanilla interacts directly with as3commons-reflect (and to a lesser extends, as3commons-lang) - this interactions should be abstracted into an interface so that Vanilla is no longer tightly coupled to a particular reflection framework - this will allow the user to write an adapter for their reflection library of choice and improve the quality & coverage of the unit tests.

Thanks to @MattesGroeger for the suggestion!

[MattesGroeger]

Eventually I can contribute a spicelib implementation then. Looking forward.

[jonnyreeves]

Ok, I've checked in the initial work for the abstraction into the 0.2 branch, Mattes I'd be interested to have your feedback on what's there so far if you have time for a review? Thanks!

[MattesGroeger]

From what I have seen it looks very well thought trough!

General structure

You basically convert all reflect informations into your own data structure (ReflectionMap). This is what you than operate on in your Vanilla main class. Here you filter out the parts Vanilla is actually interested in (e.g. [Marshall]). Please correct my if I am wrong.

While I agree that this is probably the best solution for separating the actual Vanilla logic from the reflection part it also involves a lot of conversion and object creation. You will eventually have unused reflection data if the class also uses different MetaData tags. You also restrict the way the reflection framework can work because it always has to conform to your data model. In fact all classes in the "reflection" package define your interface to the reflection api.

Idea

I was wondering if a reduced interface would maybe simplify this. What I mean is to move all the reflection related stuff from the Vanilla class in the "impl" package. Every framework adapter would than implement this in a different way. Depending on how the framework works it can be done maybe completely different. The result would then be something like your InjectionMap.

This approach would give more freedom to the different reflection frameworks adapters and would avoid the data conversion (and naming conflicts). On the down side you move a huge part of your current logic in the adapters. You would have to provide a solid way to verify the returned data.

Dependencies

Apart from this, I would also suggest to remove the AS3CommonsReflectionMapFactory() creation from the Vanilla constructor. While it is nice for a potential user to have a default implementation it also creates a coupling to the AS3Commons classes. Even if the user would use his own IReflectionMapFactory implementation it would still require the AS3Commons libs as dependency.

I would suggest to move this whole framework specific reflection part into a separate project which than has dependencies to AS3Commons and Vanilla lib. Your Vanilla lib is than completely independent because it only contains the interfaces and the remaining logic. This is the only way to make it really independent from a reflection framework. The drawback is, it adds another lib for a potential user.

Convenience method

I really like how you use the global extract() method to provide an easy access for you Vanilla class. Maybe you also find a way on how to register the framework adapter once in this scope.

Conclusion

I really appreciated browsing through your sources. You have a very good coding style and you think from a users perspective. Keep on the good work and please let me know what you think about my ideas.

[jonnyreeves]

Wow, thanks for the indepth reply Mattes, I really appreciate the time and effort you put into your reply!

Construction of the RelectionMap Yep, that's right; the current implementation expects the ReflectionMapFactory to build up the ReflectionMap and all the native classes (Parameter, Field, etc) so that Vanilla knows the makeup of the targetType. The only slight difference is that Vanilla does not want you to strip / leave anything out of the ReflectionMap - this was done so we could add additional Metadata tags in the future, should we need to.

I agree with you about the amount of excessive amount of object creation; that's the main drawback of the approach I am employing (however, I planned to counter this by the use of a cache).

Your Implementation Idea I think I understand what you are saying but I can't see how such an implementation would work - I would be really interested to see your thoughts written down, even if it was just as pseudo code - I am happy to rewrite the current implementation for dealing with abstracting the reflection layer - I just hammered it out in one evening!

If I understand you right, you are suggesting that Vanilla removes all the concrete classes from the 'reflect' package - but if we did that, how would Vanilla know how to interface / interact with the ones supplied by the chosen Reflection Library?

Dependencies Ah yeah - I just left the reference to the AS3CommonsReflectionMapFactory in Vanilla's constructor so I could get the unit tests to pass ;) I assumed that the final implementation would require you to pass a factory in. I also thought that I would change the visibility of the VANILLA_INSTANCE global so that it had public visibility and could therefore by set by the developer.

Moving the AS3Commons adapter into a seperate SWC sounds like a good plan, but yeah - having to include yet another SWC is a drag. Ultimately I think I / we (hopefully someone else!) is going to have to write a native reflector; that way Vanilla can be reduced to a single SWC - I hate reinventing the wheel but I can understand why people don't want to have to drop 4 SWCs into their project... it's a tough one :(

Thanks for the support Mattes; really appreciate it!

[MattesGroeger]

Hi Jonny,

also thanks for your detailed feedback! Also really appreciate it :)

Because I go on vacation on Wednesday, I'm a bit in a hurry. I quickly sketched the idea in my forked version (branch 0.2-idea). I didn't run the version, I'm pretty sure that certain things aren't working right now. But I hope you will get the idea.

These are my changes:

1. Got rid of the mapping classes/logic (Ctor, Field, Method, etc.)
2. The whole addReflectedRules() part from Vanilla class moved into the adapter impl
3. Created IInjectionMapSpi interface which is used by the adapter impl to store the data
4. Had to make InjectionMap public in order to access it (you could use your own namespace though)
5. InjectionMap impl uses now Vector instead of Array (better readable and I think also faster access)
6. The current AS3CommonsReflectionMapFactory impl is far from optimal, it was really fast put together

If you would like I can spend more time on it after my vacation and directly contribute to your project. But also, if you don't like what I did, please let me know.

I think the biggest drawback is, that another adapter impl. would have similar logic. To avoid this we could try to separate as much as possible from the parsing part in external utility classes. On the other hand I think the code is now more simplified.

Out of curiosity, why do you think people hesitate putting 4 SWCs in their classpath? If they already use as3Commons it's actually only 2.

Will be back on Sunday. Looking forward :)