Last night when I was working on the residue bits, I noticed there’s
some discrepancy in the new API:
Atom::deleteBond()
Molecule::deleteAtom()
… (etc)
Residue::removeAtom()
Fragment::removeAtom()
Fragment::removeBond()
Engine::removeAtom()
… (etc)
Personally, I’d probably go for “remove” unless we’re actually freeing
memory. But one way or the other, we should make a pass for
consistency in naming. I think this is the only area I picked up.
Everything else is really consistent.
If you like, I can go through with sed and fix them one way or the
other.
Cheers,
-Geoff
Geoffrey Hutchison wrote:
Last night when I was working on the residue bits, I noticed there’s
some discrepancy in the new API:
Atom::deleteBond()
Molecule::deleteAtom()
… (etc)
Residue::removeAtom()
Fragment::removeAtom()
Fragment::removeBond()
Engine::removeAtom()
… (etc)
Personally, I’d probably go for “remove” unless we’re actually freeing
memory. But one way or the other, we should make a pass for
consistency in naming. I think this is the only area I picked up.
Everything else is really consistent.
I was looking at that myself as I was cleaning up the docs for the core
classes. I think you are correct and that remove is probably the best
way to go if they are not actually being deleted. To be honest, with
some of these I am not sure if they should be protected functions and
friends. As far as I can tell add and delete bond in the atom should
only ever be called by the bond class, i.e. protected friend class possibly?
If you like, I can go through with sed and fix them one way or the other.
Sounds good. I was wondering if some of the functions should be made
protected and we should use friend classes, or if we should just
document that they should not be called directly unless you really know
what you are doing. For instance, setIndex, setId, addBond and
deleteBond in atom. They should be read only for all but the appropriate
classes.
Certainly the index and id methods could cause crashes or very hard to
diagnose behaviour if a badly behaved script messed with them for
example. I guess part of this comes down to how much we choose to use
the access control afforded to us by C++, and how much we document.
Thanks,
Marcus