Having a really hard time to grok it right now and think we can improve quite a few things.
1.) It atm only works with exactly a single InvocationHandler. That's currently a problem in my case -> We imo should split the creation of the proxy class from the creation of the proxy instance. 2.) It's hard to get what is in the API and what is an impl detail. -> We should create an interface and move the impl stuff to an internal class. Any objections? How freely can I change the API? Is it only important that our internal tests and the JPA stuff is working fine again, or do I need to care about native usages as well? I also don't get why the current DeltaSpikeProxyFactory needs the BeanManager as parameter? Maybe we could introduce a new API which just exposes a native java subclass proxy to retain backward compat? In any case we miss a lot of JavaDocs! LieGrue, strub > On Monday, 9 May 2016, 13:47, Thomas Andraschko <[email protected]> > wrote: > > a) yup > b) if it's unused, why not ;) Should be just an old case, it was refactored > many times during developed because of some AppServers / versions. > c) yup, feel free to align it :) > > > 2016-05-09 13:30 GMT+02:00 John D. Ament <[email protected]>: > >> On Mon, May 9, 2016 at 7:24 AM Mark Struberg > <[email protected]> >> wrote: >> >> > a.)DeltaSpikeProxyFactory totally misses docs it sems. Or did I miss >> > something? >> > >> >> The current docs are a step up from about two months ago when the only >> thing on the doc page was "TODO: Document the proxy module" or > something >> along those lines. >> >> Yes, there's still stuff missing. >> >> >> > >> > b.) there are methods which are imo questionable, e.g. >> > >> > public <T> Class<T> getProxyClass(BeanManager beanManager, > Class<T> >> > targetClass) >> > { >> > return getProxyClass(beanManager, targetClass, >> > DummyInvocationHandler.class); >> > } >> > >> > >> > what do we need this for? Can I simply remove it? >> > The DummyInvocationHandler just returns null. Without calling the > proxied >> > instance. It's basically a no-op impl. Why do we need this? >> > >> >> This smells like a strangle that wasn't completed. I'd say if it > can be >> delegated down, lets remove it. Better is if its only used in tests. >> >> >> > >> > >> > c.) The ordering of the methods are mixed. Imo all public methods > should >> > be on top, protected and private at the bottom. Really hard to read > atm. >> > >> >> +1 >> >> >> > >> > Any thoughts? >> > >> > LieGrue, >> > strub >> > >> >
