Re: [morph-developer] recent changes
Brought to you by:
orangeherbert,
sgarlatm
From: Matt B. <gud...@ya...> - 2007-01-13 15:28:21
|
Responses inline: --- Matt Sgarlata <Mat...@wh...> wrote: > Wow I'm slow getting back to you >< All these > changes look great! I'm > very glad you have SVN access for this stuff :) > Thanks. :) > Here are some minor changes I made: > - ReflectionInfo class, took your change of > !isLowerCase and further > refined it to be isUpperCase. Also applied the > change in multiple > places (you only caught 1 of the 3 places the check > happened) I probably outsmarted myself there; I was thinking of something like set7foo or something but since 7foo wouldn't be a valid property (member) name anyway it really doesn't matter. :) > - NumberToBooleanConverter, added comment indicating > .equals cannot be > used (to prevent someone going in there and messing > it up in the future) > - ObjectToClassConverter, added comment explaining > why > isTransformableImpl was implemented (for > performance) > > Regarding the MultipleDestinationConverter, the > final call in the class > was probably supposed to be > getContainerConverter().convert(destinationClass, > *destinationObjects*, > locale); I went ahead and made this change. This > class was created > because someone was challenging me that my > interfaces only allowed 1 -> > 1 object conversions as opposed to also allowing one > object to be broken > into several smaller object or several smaller > objects to be combined > into 1 larger object. The idea here would be you > had something like, > say, a Person that you wanted to be converted into a > collection of > smaller objects like PoliticalViews, BankAccount, > Cars. Those 3 smaller > objects would have to be returned as some type of > collection, say as a > List or an array of Object[]s. That was what the > final call to > getContainerConverter() was for. Really this is > more of a hypothetical > class to show that this type of thing was possible > to my friend I was > hoping would get involved in the project (didn't > work). I don't > anticipate anyone would actually use this class, and > I probably haven't > even looked at it since the date it was first > written. Perhaps we > should just delete it? > No, I think it has a place. There's some Fowlerism like "Assembly Pattern" or something that this should be an example of... Dozer explicitly mentions it in their manual, so you're probably best off keeping it for competitive reasons. We should just try to document it better and provide examples. Does this mean CombiningCopier was meant as the opposite of MultipleDestinationConverter? > Question: why did you put in clone() calls in the > TimeConverter, if the > source class = destination class? By way of > compmarison, the > IdentityConverter does not attempt to make copies of > source objects and > just returns them unchanged. The tradeoff as I see > it is that clone() > is "safer" but just returning the source unchanged > is faster. Since we > can't universally clone objects (since not all > objects implement that > interface), would it be best to just never do > cloning of source objects? Here are my thoughts: you can't just return the original instance when the source class is Date or Calendar, because both classes are mutable. If they weren't they could just have been included in the IdentityConverter's list of source/dest classes... since they are mutable you want (IMO) to break their existences apart so that they can have individual identity... kind of the very concept of "clone". And I don't see anything wrong with a converter that knows it operates on a finite number of classes, and that all of those classes implement Cloneable (it's highly unlikely that will be retracted in the future), making use of that knowledge. > > Suggestion: how about we rename Int to > MutableInteger, to more > accurately describe what it is for? This would also > preclude confusion > regarding what is returned by the Morph.convertToInt > method. > This is fine with me. It's funny because some of the test cases rely on stuff like commons-lang, which, of course, has a MutableInteger already. Although this Int is probably less encapsulated since I left the value member public for quick/easy use. > FYI: I made some improvements to > MorphPropertyEditor, which I am > actively using in one of my apps to customize > Spring's data binding > behavior (I happen to need some behavior that is not > Morph's default, > specifically related to the TextToNumberConverter) Cool. -Matt B > > Matt > > Matt Benson wrote: > > Matt S (and anyone else who may be watching): > > > > I have been working in the Morph codebase > yesterday > > and today. Highlights of what I've done: > > > > r10 | matt.benson | 2007-01-08 13:38:04 -0600 > (Mon, 08 > > Jan 2007) | 3 lines > > > > Fix/improve date-related test failures by sticking > to > > Calendar instead of Date > > whenever possible. > > > > -Allowed me to establish a baseline of > all-tests-pass. > > > > r11 | matt.benson | 2007-01-08 13:49:02 -0600 > (Mon, 08 > > Jan 2007) | 2 lines > > > > Fix ChainedConverter so that it will work > correctly > > for ExplicitTransformers. > > > > -The original bug I saw that I thought I could > fix. > > > > r17 | matt.benson | 2007-01-08 16:38:16 -0600 > (Mon, 08 > > Jan 2007) | 1 line > > > > make text converter handle byte and char arrays > > > > -Seemed like the right thing to do. > > > > r18 | matt.benson | 2007-01-08 16:48:00 -0600 > (Mon, 08 > > Jan 2007) | 1 line > > > > delegate text to boolean conversion using a > > textconverter, thus gaining any additional source > > classes added to textconverter for free > > > > -followed naturally from the previous change. > > > > r19 | matt.benson | 2007-01-08 16:51:31 -0600 > (Mon, 08 > > Jan 2007) | 1 line > > > > fix broken stack management and add object to > pretty > > text converter test case > > > > -ObjectToPrettyTextConverter had no test case and > > didn't work properly. :( > > > > r29 | matt.benson | 2007-01-08 17:22:06 -0600 > (Mon, 08 > > Jan 2007) | 2 lines > > > > add missing primitives; quicker > isTransformableImpl(); > > eliminate code duplication > > > > -I couldn't think why ObjectToClassConverter > allowed > > all primitives as sources except for boolean and > char, > > so I added them. Since this converter's only > > destination Class is Class, and Class is final, I > > overrode isTransformableImpl to check whether the > > destinationType were Class.class... > > > > r32 | matt.benson | 2007-01-08 17:30:32 -0600 > (Mon, 08 > > Jan 2007) | 1 line > > > > add UnsupportedOperationException to > > CombiningCopier.copyImpl() > > > > -not yet implemented > > > > Additionally I made several changes having to do > with > > eliminating redundant (explicitly or logically) > code, > > reducing method calls, object instantiations, > source > > file size, etc. where I saw possibilities. Some > of > > these could be considered style issues so I hope I > > haven't overstepped any bounds here. > > > > br, > > Matt B > > > > __________________________________________________ > > Do You Yahoo!? > > Tired of spam? Yahoo! Mail has the best spam > protection around > > http://mail.yahoo.com > > > > > ------------------------------------------------------------------------- > > Take Surveys. Earn Cash. Influence the Future of > IT > > Join SourceForge.net's Techsay panel and you'll > get the chance to share your > > opinions on IT & business topics through brief > surveys - and earn cash > > > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > > _______________________________________________ > > morph-developer mailing list > > mor...@li... > > > https://lists.sourceforge.net/lists/listinfo/morph-developer > > > > > > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get > the chance to share your > opinions on IT & business topics through brief > surveys - and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > morph-developer mailing list > mor...@li... > https://lists.sourceforge.net/lists/listinfo/morph-developer > ____________________________________________________________________________________ Any questions? Get answers on any topic at www.Answers.yahoo.com. Try it now. |