From: Matt S. <Mat...@wh...> - 2007-02-05 21:35:32
|
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 > > |