Patch/apicleanup2 #309
Patch/apicleanup2 #309
Conversation
|
Can you please rebase? |
|
(Also: is this to be put in before the paper resubmission?) |
|
No need before resubmission but before 2.0 release |
|
Rebased |
|
Not sure about the method name getBeg()... I'm personally not in favor of this... getBegin(), getStart() is only marginally longer... I fully support this as short cut for getAtom(0)... no matter what the name is, it nicely reflects the assumption of a two-atom bond... |
|
I'm okay with getBegin()... OEChem's getBgn() is horrible maybe we ask the mailing list. The reason I used getBeg() it because looks marginally nicer than |
|
I may also add a setBegin/setEnd but actually a water-tight API would say you must create a new bond rather than modifying the end points. In all cases I looked at bond.setAtom() is only used before a bond is added to an IAtomContainer hence it may as well be immutable... of course the current API doesn't allow that. |
… than the whole array
…memcpy (List[]->IAtom[]).
|
rebased |
|
Awesome and thanks - I'm going to do a CDK 1.5.15 release ahead of 2.0. I've almost got the fast AtomContianer impl working but there will be some breakages if the API is currently being miss-used. Essentially the atom/bond you put into the AtomContainer isn't the one you get back, you get back a wrapped reference (which can choose to unbox). Any unchecked casts to non CDK interfaces (IAtom, IPseudoAtom, IQueryAtom) will now fail, the only place this was a problem in the tests was the pcore matching. If the API is being used safely (no unchecked casts or reference equality) then the new API will work fine but to be portable I'll do a 1.5.15 that will allow Bioclipse/AMBIT the upgrades with no unexpected behaviour. |
Clean up two, I've left the n-centre bonds as they are (for now) but added utility methods for
bond.getAtom(0)andbond.getAtom(1). These can now be called withbond.getBeg()andbond.getEnd(). Also ```getConnectedAtom(IAtom)is now the shortergetOther(IAtom)``. The existing methods are still there so no API breakage.The first one is shortened to line up nicely.
The terms begin/first, and end/second are widely used, not only do they provide cleaner code but they should lower the barrier to entry from other toolkits. Here is a quick survey of other toolkits.
OEChem: bond.GetBgn() and bond.GetEnd() bond.GetNbr()
RDKit getBeginAtom() getEndAtom() getOtherAtom()
OpenBabel bond.GetBeginAtom() bond.GetEndAtom() bond.GetNbrAtom()
ChemAxon: bond.getAtom1() bond.getAtom2() bond.getOtherAtom()