Menu
▾
▴
Re: [Cdk-devel] Interface change request for IAtomContainerSet
|
From: Egon W. <ego...@gm...> - 2009-10-14 11:59:02
|
On Wed, Oct 14, 2009 at 2:35 PM, Mark Rijnbeek <ma...@eb...> wrote:
> you could do that, but the scope of "atomContainers" in AtomContainerSet
> now is protected, so I'm not sure if AtomContainerSetManipulator is
> allowed to sort.
Ah, good point. The use of Comparator is indeed closely linked to the
implementation...
Mmm... that also means that bringing in this method brings
implementation details to the API... :(
That said, there was serious talk earlier about using
List<IAtomContainer> anyway... But it would have to go in one rewrite,
I'd say...
> I just thought to add method
> public void sortAtomContainers( Comparator<IAtomContainer> comparator);
> to interface IAtomContainerSet, because that is where I need it.
If the implementation does not use List<IAtomContainer> internally, it
can still use the Comparator interface to do the sorting... it just
can't take advantage of Arrays.sort()...
OK, yes, I guess putting it in the interface is a reasonable trade
off... while suggesting some internal implementation type, it does not
remove the option to do otherwise...
What do others think? Anyone seeing a better solution?
> And then implement it in AtomContainerSet, using one line really:
> public void sortAtomContainers( Comparator<IAtomContainer> comparator)
> {
> Arrays.sort(atomContainers, comparator);
> }
>
> The question is also in which package developer should keep the
> implemented Comparators, it would be nice to have them in one place.
cdk.tools.comparators ?
> My comparator:
> class Comparator2DCenter implements Comparator<IAtomContainer> {
> public int compare(IAtomContainer atCont1, IAtomContainer atCont2) {
> Point2d p1= GeometryTools.get2DCenter(atCont1);
> Point2d p2= GeometryTools.get2DCenter(atCont2);
> if (p1.x!=p2.x) {
> return new Double(p1.x).compareTo(new Double(p2.x));
> }
> else {
> return new Double(p1.y).compareTo(new Double(p2.y));
Is there not some convenience method in the vecmath library to do this...
> }
> }
> }
I'm looking forward to the full patch as you suggest...
Egon
--
Post-doc @ Uppsala University
Homepage: http://egonw.github.com/
Blog: http://chem-bla-ics.blogspot.com/
PubList: http://www.citeulike.org/user/egonw/tag/papers
|