On Tue, 19 Mar 2002, Morgan Delagrange wrote: > > I disagree. a no-arg constructor and dynamic instantiation of the > > comparator may be useful without a get/setComparator mechanism. For > > example, consider an application that allows you to specify a comparator > > in a configuration file while it will use for a particular application > > specific task. The config fail may just ask for the comparator class > > name, and the app can instantiate an instance of the comparator to use. > > I don't dispute that the new no-op constructor is an improvement. :) But > if we're going to fix it, let's fix it right...I'm not sure we've gone far > enough yet. Right now, the only use for that no-arg constructor is to > provide functionality identical to Collections.reverseOrder(). That doesn't > seem very useful at all. If you want to support bean-like construction, > then let's add getter and setter methods; if not, then let's dump the no-arg > constructor entirely. This middle ground seems inadequate. I'd just pick > one, but I don't want to tick anybody off.
Having the no-arg be equivalent to Collections.reverseOrder is still useful -- there's no easy way to specify the comparator returned from Collections.reverseOrder() as a configuration option in, say, an digestor based configuration file: <field name="name" comparator="org.apache.commons.collections.comparators.ComparableComparator" > <field name="date" comparator="org.apache.commons.collections.comparators.ReverseComparator" > That's the big advantage that makes me want to keep the no-arg even though equivalant functionality exists in Collections.reverseOrder. > Here's my case for just removing the no-arg comparator. 1) Bean-like > construction of a Comparator seems unimportant as a feature. 2) If only the > constructor can set the comparator that's reversed, it's not possible to do > something naughty like changing a comparator after it has already been > assigned to a SortedXXX. 1) its important for the use-case above. 2) having a no-arg doesn't imply the comparator can be changed after assigned to a SortedXXX. It seems like both of these arguments *do* apply against adding a setComparator method though: it seems unimportant as a feature to be able to change the underlying comparator (rather than just creating a new one), and changing the functionality of the comparator after its assigned to a SortedXXX is a Bad Thing. I should also mention that a no-arg constructor does not imply that the comparator is a "bean" and needs to have get/set methods for all its attributes. And even if it does, I would still argue that the comparator is would not be a read/write attribute. I'd be ok with a get method if that makes things more "bean-like". > If folks really want no-arg, then let's at least add a setter method. For > me, that option is a distant second, but livable. I will -1 adding a setComparator method. I think its too dangerous to allow the comparator to change behavior after it has been constructed. > > get/setComparator methods are really only useful for altering the > > underlying comparator once the reverse/inverse comparator has been > > constructed. I think this is bad because the underlying comparator is > > part of the functionality, and providing modification of it thus changes > > the functionality of the reverse/inverse comparator. That doesn't seem > > like an appropriate attribute modification. > > Definitely agree. I'd prefer to not have getter/setter methods. ok, so we agree. :) > > Since there's been a bit of discussion on the issue, and I haven't really > > heard any objections to my diff (fixing the comparator with respect to the > > comparator contract), I'm going to check that in. I'm also going to check > > in the argument swap rather than multiplication. We can continue > > discussion on the rename and whether or not to have a no-arg constructor. > > -0 for a no-arg constructor. This is weird actually. That no-arg has been in there since the class was added in February, and probably before that while the class existed in the util sandbox. Status-quo in this case is to leave the no-arg constructor. My changes were merely to "fix" the no-arg so that it conforms to the Comparator contract. Makes for some interesting apache style voting. :) > -1 for the name change, only because it has the same connotation as and > shares terminology with Collections.reverseOrder(). Might as well be > consistent. If the term seems awkward, then a more specific definition in > the Javadoc should suffice. Fine by me. After removing the multiplicationg in favor of just reversing the elements, it now seems just as appropriate to calling it a ReverseComparator as it does to call it an InverseComparator. ;) regards, michael -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
