On Wed, Dec 3, 2008 at 12:14 PM, Adam Winer <[EMAIL PROTECTED]> wrote:
> On Wed, Dec 3, 2008 at 10:36 AM, Paul Lindner <[EMAIL PROTECTED]> wrote: > > I was the one that put in the @ImplementedBy annotations and I stand by > > them, especially for Model classes. Not everyone coming into this > project > > is a Guice expert, and maintaining your own Module was a royal pain in > the > > ass before we put in @ImplementedBy. > > > > The other big big problem is that guice Modules are Atomic, and you can't > > replace an existing binding (that throws an error). Let's say you want > to > > implement a single model class, say 'Person'. To do this you have to: > > > > 1) Make a copy of the existing shindig module > > 2) Modify the Person binding to use your own class. > > 3) Maintain this module forever. > > > > This is bad. You now have to merge in any code changes to this module > into > > your own customized module... forever. Someone tell me that this is > fixed > > in Guice 2.0. > > Yes, Guice 2.0 has good support for overriding modules. > > http://publicobject.com/2008/05/overriding-bindings-in-guice.html > > > The problem with XStream081Configuration is just a design flaw and should > be > > fixed. Isn't there a Guice specific way to identify the proper > > implementation class for an interface? > If this is a one time setup, couldn't you just do: injector.getInstance(InterfaceName.class).getClass() assuming the instance is zero cost. Alternatively, why not bind the classes themselves using a map? > This problem crops up in other > > unmarshaling code too, for example JAXB has this same problem when > creating > > classes from XML. At least Guice has an explicit binding here, so I > think > > it should be possible for us to fix this and use the proper > implementation > > classes. > > Indeed. The XStream code needs to be fixed. This will be easier in > Guice 2.0, which adds introspection APIs. > > > > > > > > > On Dec 3, 2008, at 10:08 AM, Henning P. Schmiedehausen wrote: > > > >> So we are integrating Shindig into our own code base. Which is large, > >> complex and I try to keep a few personal guidelines that go against > >> the "current industry trends" but 25 years of computer programming > >> should be good for something. :-) One of them is "everything that > >> happens automatically is bound to break". Always. > >> > >> What we want to do is provide our own implementations of some > >> opensocial model classes. There is a reason for this, mainly because > >> we have more fields in compliance with > >> > >> > http://www.opensocial.org/Technical-Resources/opensocial-spec-v081#TOC-Compliance > >> using <prefix>.specialField. Which are represented by additional > >> fields in our objects. > >> > >> (Which BTW leads the whole idea of using getter/setter based bean > >> classes for Shindig ad absurdum. The current model classes clamp down > >> exactly what is in the spec without allowing the dynamic extension > >> mechanisms shown there. Using Enums is equally bad, because extending > >> these leads to necessary Shindig code modifications. But let's not go > >> there). > >> > >> And it breaks. It breaks in a very puzzling way. Because at some > >> point, the PersonImpl classes started to bleed into our code, even > >> though our custom GuiceModule contains > >> > >> bind(Person.class).to(MyCustomPerson.class); > >> > >> The reason for this is the following piece of code in > >> InterfaceClassMapper. A class that can only be replaced by ripping out > >> the whole XStream081Configuration class, because it is hard coded in > >> there. > >> > >> --- cut --- > >> private Class<?> getImplementation(Class<?> clazz) { > >> // get the guice default implementation on the class. > >> // we might use the injector to discover this > >> Class<?> cl = clazz; > >> ImplementedBy implementedBy = clazz.getAnnotation(ImplementedBy.class); > >> if (implementedBy != null) { > >> Class<?> c = implementedBy.value(); > >> if (log.isDebugEnabled()) { > >> log.debug("===================Class " + clazz + " is implemented by > " > >> + c); > >> } > >> cl = c; > >> } else { > >> if (log.isDebugEnabled()) { > >> log.debug("===================Class " + clazz > >> + " no implementation, assume concrete "); > >> } > >> } > >> return cl; > >> } > >> --- cut --- > >> > >> This method *TIES* the @ImplementedBy implementations of the model > >> interfaces to the interfaces. And you can not override this without > >> recompiling Shindig. So why use IoC at all? And argueing that this > >> *helps* implementors to understand how Shindig works is simply > >> wrong. This is error-prone, difficult to read and understand and the > >> current code structure makes it *harder* for implementors to > >> understand where they can and must tweak the code base for integrating > >> it into their project. > >> > >> So I beg to differ to Ian: @ImplementedBy makes the code base harder > >> to read and understand. Because there is no centralized point of entry > >> where the parts are tied together. Which was (up until now) my > >> understanding why people are using IoC containers in the first place. > >> > >> Why you have Modules in Guice and config files in Spring. But that is > >> just me. > >> > >> Ciao > >> Henning > >> > >> -- > >> Henning P. Schmiedehausen - Palo Alto, California, U.S.A. > >> [EMAIL PROTECTED] "We're Germans and we use Unix. > >> [EMAIL PROTECTED] That's a combination of two demographic > groups > >> known to have no sense of humour whatsoever." > >> -- Hanno Mueller, > de.comp.os.unix.programming > > > > Paul Lindner > > [EMAIL PROTECTED] > > > > > > > > >
