On Wed, Dec 3, 2008 at 1:57 PM, Henning P. Schmiedehausen <[EMAIL PROTECTED]> wrote: > Adam Winer <[EMAIL PROTECTED]> writes: > >>> 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 > > Ah, now I see your (and Kevin's and Paul's) reasoning: > > You are argueing that e.g. DefaultGuiceModule and SocialApiGuiceModule > are good enough to be used in production environments. Basically, the > stuff set up in there is golden and can be used everywhere.
No, that's the opposite of the point. If it was, then @ImplementedBy never would have been used, since a large quantity of boilerplate would just go in those modules, and no one would need to override them. The existing modules are far from perfect, and can't be used in production. SocialApiGuiceModule referencing samplecontainer code immediately disqualifies it. And "DefaultGuiceModule" - which should be named GadgetsModule - almost certainly shouldn't supply its own Executor/ExecutorService. > This is where I disagree. I consider the various parts of Shindig > components that are put together by a frame which uses Guice and turn > them into an app. > > We e.g. use a very different frame to turn the Shindig components into > a working opensocial container. And I use the supplied Guice modules > solely as a reference on which components must be > implemented/touched/used. And finding this reference is very hard with > it being spread out all over the code base. > > Guice modules are configuration files. Treat them like this. In part, yes. But no one should have to set up a large - and, when @ImplementedBy was introduced, constantly changing! - Guice module just to use a library, anymore then you'd hand them a couple hundred lines of config files and say "start from here". > > Ciao > Henning > > > >>> 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? 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] >>> >>> >>> >>> > > -- > 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 >
