From: Matt B. <gud...@ya...> - 2007-02-05 22:06:35
|
I can see your point here, and concede that it is possible to continue as-is. I guess I am aiming for trying to make the experience of working with Morph one of writing tiny classes to do very specific things which can then be arranged by configuration to accomplish the large tasks. So when we see something as common as "how do we get the destination instance for a conversion" that makes an attractive extension point. I suppose it would be possible to add the parent converter concept to BaseTransformer without using a new interface--the default converter could delegate to the BaseTransformer instance's (Instantiating-) Reflector so as to preserve the status quo while permitting the granular customization I am after... would this sound like a reasonable compromise to you? -Matt B --- Matt Sgarlata <Mat...@wh...> wrote: > Hello again Matt B - thanks for following up on this > issue, because I > think it's important we get this right. One of the > main benefits of > Morph over other frameworks is the strength of its > abstractions, and > right now it seems like Morph is falling flat on its > face when dealing > with objects that don't have a public no-arg > constructor. > > I see what you're getting at with the > DerivedConverter, but I'm not > convinced that a new interface is needed here. What > should be happening > to avoid use of the InstantiatingReflector is simply > overriding the > convertImpl method to do the desired conversion > operation. I know the > default implementation looks scary: > > Object reuseableSource = > createReusableSource(destinationClass, > source); > Object newInstance = > createNewInstanceImpl(destinationClass, > reuseableSource); > copyImpl(newInstance, reuseableSource, > locale, > Converter.TRANSFORMATION_TYPE_CONVERT); > return newInstance; > > but overriding it isn't nearly so horrid: > > Object newInstance = new ComplexObject(arg1, > arg2, arg3); > copyImpl(newInstance, source, locale, > Converter.TRANSFORMATION_TYPE_CONVERT); > return newInstance; > > I really think the problem here is just the comments > on the convertImpl > method. The documentation states "this > implementation should be fine > as-is for Copiers" when instead it should state "you > will need to > override this method if your destination object does > not have a public > no-arg constructor". > > In terms of strengthening abstractions for the > AssemblerCopier and > DisassemblerCopier, I think that's an issue for > those copiers to > address, not for BaseTransformer. > > Matt S > > Matt Benson wrote: > > Okay... my design might be somewhat crippled by > the > > attempt to be as non-disruptive as possible: > > > > /** A converter derived from another converter */ > > interface DerivedConverter extends Converter { > > Converter getParentConverter(); > > } > > > > Basically I think BaseTransformer would provide > the > > DerivedConverter implementation something like > this: > > > > public class BaseTransformer ... { > > private static final Converter DEFAULT_PARENT = > new > > NewInstanceConverter(); > > > > private Converter parentConverter; > > > > public Converter getParentConverter() { > > return parentConverter == null ? > DEFAULT_PARENT : > > parentConverter; > > } > > > > public void setParentConverter() { //routine } > > > > protected Object convertImpl(Class > destinationClass, > > Object source, > > Locale locale) throws Exception { > > if (!(this instanceof DerivedConverter)) { > > throw new UnsupportedOperation(); > > } > > Object reuseableSource = > > createReusableSource(destinationClass, source); > > Object destObject = > > getParentConverter().convert(destinationClass, > > reuseableSource); > > copyImpl(destObject, reuseableSource, locale, > > Converter.TRANSFORMATION_TYPE_CONVERT); > > return destObject; > > } > > } > > > > Again, we are talking ultimately about an entity > that > > transforms a source object to an instance of a > > specified destination class. If we do away with > the > > InstantiatingReflector concept, AT LEAST where > > converters are concerned, Morph becomes more > > versatile, because any converter can be used as > the > > parent converter in a derivedConverter > implementation. > > A concrete example: > > > > Morph contains a (-n unfinished) > > MultipleDestinationConverter. This can also be > viewed > > as a DisassemblerCopier (complemented by my > recently > > added AssemblerCopier). In terms of implementing > > copying, this is pretty simple. You have a list > of > > copier children which should be the same size as > the > > destination object container. The component > copiers > > are invoked one at a time from the source object > to > > the corresponding child in the destination > container. > > The existing partial MultipleDestinationConverter > > implementation uses a list of classes to tell it > how > > to create the destination object when implementing > > Converter. But this is still a case of "we know > how > > to copy and we just need to know how to create the > > destination object". InstantiatingReflector > doesn't > > fully represent the concept of creating a > (probably > > heterogenous) collection of destination objects > IMHO. > > But a parent Converter is semantically open-ended > and, > > I believe, describes both situations fairly: the > > simple (.newInstance()) as well as the complex. > > > > Does that make anything any clearer? > > > > -MJB > > > > --- Matt Sgarlata > > <Mat...@wh...> wrote: > > > > > >> I agree, the addition of the "Object parameters" > >> parameter to > >> InstantiatingReflector.newInstance is worrisome. > >> The idea behind a > >> Reflector in general is that it tells you how to > >> work with some new type > >> of object that has properties which are not known > >> until runtime. For > >> example, Maps, DynaBeans, ResultSets, etc. If > you > >> have some sort of > >> domain object there should be no need to write a > >> reflector, because > >> Morph's built-in ObjectReflector can use Java's > >> built-in reflection > >> capabilities to determine the properties of the > >> object. If you just > >> want to be able to convert stuff, it should be > >> sufficient to write a > >> single Transformer that implements both Converter > >> and Copier interfaces > >> and doesn't mess with InstantiatingReflectors at > >> all. > >> > >> That said, I think the > >> InstantiatingReflector.newInstance method is > >> something that should be present in the API and > not > >> deprecated. We can > >> and should specify in the comments what this > method > >> should and should > >> not be used for. However, it is possible that > some > >> types of dynamic > >> objects would require extra information before > they > >> are created. For > >> example, the BasicDynaBean has no default > >> constructor and requires a > >> DynaClass to be constructed. Its properties are > not > >> known until > >> runtime, but the DynaClass provides a contract > >> describing the object's > >> properties, etc. > >> > >> So in summary, I'm saying I think we have a > >> documentation issue not an > >> API issue. However, I don't fully understand > what > >> API changes you are > >> proposing, so could you post some pseudo code for > us > >> to look at? > >> > >> Thanks, > >> > >> Matt S > >> > >> Matt Benson wrote: > >> > >>> I am reawakening this thread. I touched on this > >>> subject during the original discussion, but at > the > >>> time I couldn't quite articulate what was > >>> > >> bothering me > >> > >>> about the whole business. Last Friday it burst > >>> > >> upon > >> > >>> me, and I have continued to kick it around over > >>> > >> the > >> > >>> weekend but have not been able to refute my own > >>> > >> logic: > >> > >>> Basically with InstantiatingReflector taking a > >>> > >> source > >> > >>> parameter, it IS a converter. What is the > >>> > >> difference > >> > >>> between: > >>> > >>> InstantiatingReflector.newInstance(Class, > Object) > >>> -and- > >>> DecoratedConverter.convert(Class, Object) > >>> ? > >>> > >>> >From my POV, nothing. Different semantics for > >>> > >> what, > >> > >>> using the method parameters as an indicator, is > >>> > >> one > >> > >>> concept. The role of the InstantiatingReflector > >>> > >> in > >> > >>> the Morph library is to adapt a copier to the > >>> converter interface, using the following > >>> > >> algorithm: > >> > >>> 1. Create a new instance I > >>> 2. Invoke copy() using I as the destination > >>> > >> object. > >> > >>> This sequence of operations amounts to a > "derived > >>> conversion": one which relies on a parent > >>> > >> conversion > >> > >>> to supply the target object, then modifies the > >>> resulting object. This modification will always > >>> consist of making changes on a target object > based > >>> > >> on > >> > >>> the source object (otherwise the parent > converter > >>> alone would suffice), and thus infallibly > >>> > >> corresponds > >> > >>> in intent with Copier.copy(). This proves that > a > >>> DerivedConverter is precisely a Copier that > >>> > >> implements > >> > >>> Converter by first delegating to a parent > >>> > >> Converter, > >> > >>> then copies the source to the resulting > >>> > >> destination > >> > >>> object. The parent converter can be overridden, > >>> > >> but > >> > >>> would default to an, e.g., NewInstanceConverter > >>> > >> which > >> > >>> simply creates a new instance of the > >>> > >> destinationClass. > >> > >>> In adopting the DerivedConverter concept, > >>> InstantiatingReflector would, in my opinion, no > >>> > >> longer > >> > >>> be needed, and would hopefully be deprecated and > >>> > >> then > >> > >>> removed as soon as the current players on the > >>> > >> mailing > >> > >>> list(s) are all up to speed. Because the > >>> > >> interfaces > >> > >>> are so similar, I would expect the refactoring > of > >>> > >> a > >> > >>> custom InstantiatingReflector implementation to > a > >>> custom Converter implementation to be trivial. > >>> > >>> I invite discussion, but I really think I have > >>> something here. > >>> > >>> Impatiently awaiting comment, > >>> Matt B > >>> > >>> > >>> > >>> > >>> > > > ____________________________________________________________________________________ > > > >>> We won't tell. Get more on shows you hate to > love > >>> (and love to hate): Yahoo! TV's Guilty Pleasures > >>> > >> list. > >> > >>> http://tv.yahoo.com/collections/265 > >>> > >>> > >>> > > > ------------------------------------------------------------------------- > > > >>> Using Tomcat but need to do more? Need to > support > >>> > >> web services, security? > >> > >>> Get stuff done quickly with pre-integrated > >>> > >> technology to make your job easier. > >> > >>> Download IBM WebSphere Application Server > v.1.0.1 > >>> > >> based on Apache Geronimo > >> > > > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 > > > >>> _______________________________________________ > >>> morph-developer mailing list > >>> mor...@li... > >>> > >>> > > > https://lists.sourceforge.net/lists/listinfo/morph-developer > > > >>> > >>> > >> > >> > > > ------------------------------------------------------------------------- > > > >> Using Tomcat but need to do more? Need to support > >> web services, security? > >> Get stuff done quickly with pre-integrated > >> technology to make your job easier. > >> Download IBM WebSphere Application Server v.1.0.1 > >> based on Apache Geronimo > >> > >> > > > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 > > > >> _______________________________________________ > >> morph-developer mailing list > >> mor...@li... > >> > >> > > > https://lists.sourceforge.net/lists/listinfo/morph-developer > > > > > > > > > > > > > > > ____________________________________________________________________________________ > > Yahoo! Music Unlimited > > Access over 1 million songs. > > http://music.yahoo.com/unlimited > > > > > ------------------------------------------------------------------------- > > Using Tomcat but need to do more? Need to support > web services, security? > > Get stuff done quickly with pre-integrated > technology to make your job easier. > > Download IBM WebSphere Application Server v.1.0.1 > based on Apache Geronimo > > > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 > > _______________________________________________ > > morph-developer mailing list > > mor...@li... > > > https://lists.sourceforge.net/lists/listinfo/morph-developer > > > > > > > ------------------------------------------------------------------------- > Using Tomcat but need to do more? Need to support > web services, security? > Get stuff done quickly with pre-integrated > technology to make your job easier. > Download IBM WebSphere Application Server v.1.0.1 > based on Apache Geronimo > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642> _______________________________________________ > morph-developer mailing list > mor...@li... > https://lists.sourceforge.net/lists/listinfo/morph-developer > ____________________________________________________________________________________ TV dinner still cooling? Check out "Tonight's Picks" on Yahoo! TV. http://tv.yahoo.com/ |