On 18. Jul. 2006, at 14:06, Mike Schrag wrote:
> I just wrote this to Marc directly,
me2 :)
> I've had to rethink the approach a little ... Andrus wasn't keen on
> the patch, and he had some good reasonings behind it. For the
> record, his claims about Lists is something he is totally right
> about and a bug that I actually introduced into your original code
> when I was trying to be Mr. Clever. Here is the bug report:
> http://issues.apache.org/cayenne/browse/CAY-600
From that Bug report:
> 1. PropertyListComparator. It can only handle keys of the same
type, and throws exception if the keys are not the same and/or the
keys are not of one of the hardcoded types. We need something more
generic that can gracefully handle any kind of key
Sure. For the eomodels I didn't need it yet, and it didn't seem
obvious how to do it, so I just left the Exception in and decided to
do it when I hit that exception.
I took a quick glance at your code. It seems you are comparing
toString() when everything else fails. I'm not sure if the compare
algorith is still stable (from a<b and b<c it has to follow a<c). I
introduced a similar bug when I put in the special case for comparing
the "name" key first, and then the rest of the keys.
> 2. Sorting a list seems wrong - list is not a random order data
structure
Yep. Unfortunately it didn't make much sense do not sort lists (as I
wrote in my other email). The question is, which lists are
semantically sets, and which aren't. I don't know a real solution to
this.
> 3. PropertyListComparator comparing Maps is probably added to
handle case (2). By itself it seems redundant and wrong.
Don't know what this is supposed to mean. I wanted to compare two
dictionaries so their "name" key is handled first, and if that fails,
their sorted keys and values are compared.
> So basically the disagreements he had were related to the code
> being a little too specialized, which is kind of true -- it was
> obviously designed with EOModel plists in mind. It also was
> discussed whether sorting when writing was correct, or whether it
> should just preserve whatever order the model presents it with. As
> a result, I switched things around and redesigned how the plists
> are READ.
My goal was that all emodel files are always in normalized form. When
you only sort when reading, but not when eg. manipulating with the
EOModel editor, you would get an unsorted file on write? That
shouldn't be the case.
> So now I read the plist OUT into sorted data structures by
> modifying his Parser to take a factory that I can return TreeMap
> instead of HashMap, for instance.
Ah. If you are using TreeMap, my last remark does not apply of course.
Btw, what comparator does TreeMap use? How does it compare Maps
(dictionaries) as keys?
> This also means that the PropertyListComparator moves into
> EOModeler and out of Cayenne, so it's OK then that it is a little
> more specialized. I did end up adding support for comparing
> incompatible types, though, because UserInfo can actually have a
> mix of String and Number keys (as an example). So the basic change
> is that the plist when loaded out of the is actually sorted
> according to your rules, and when I write it, it just writes in the
> natural order of the Map it's given (which happens now to be sorted).
That sounds fine. Except I don't understand where
PropertyListComparator is being used now, as you are using TreeMap now.
So two problems remain. What to do about lists (to sort or not to
sort), and how to have a stable comparison between different types of
objects.
Marc
This archive was generated by hypermail 2.0.0 : Tue Jul 18 2006 - 08:39:54 EDT