Daniel - thanks for your good review! I'll make the suggested changes. I agree with everything on Rotation3D and Matrix3D. Regarding ParticleFunction - see discussion below. On Nov 11, 2008, at 8:58 AM, Daniel Russel wrote:
> Rotation3D: > - the protected methods should probably be private as nothing is > likely to inherit from Rotation3D and doxygen skips documenting > private methods and the methods are not general purpose > > - it would be nice to be able to initialize from a vector and an angle > - the current constructor should probably say Euler angle somewhere in > its docs.
> > > Matrix3D: > - why are there the rotation methods there? A cleaner interface would > probably be to have a method is_rotation() and another get_rotation() > so you can check is is close to a rotation and then get the rotation3D > object out directly. Having to go through intermediates of some sort > is messy. > - it needs an operator[] which returns a vector3D (with appropriate > bounds checking and all that). > - I would suggest using \brief to define multi-line brief comments > > ParticleFunction: > - Why does ParticleFunction take Particles? If you want that it should > be ParticlesFunction, but it seems to me what we really want is for it > to only take one particle and then use apply methods if we want to > apply it to more than one. Otherwise we will be creating Particles > with one Particle everywhere. yes ... , but then I'll have to do a loop each time I want to rotate a molecule. maybe it would be better to have MolecularHierarchyDecoratorFunction ? So I could just give as input the root of the molecule? > > - apply should be pure virtual so that you can tell if you got the > name or signature wrong when overriding it. sure. > > - at least one of the methods needs to be defined in a .cpp to get > linking and casting right when using multiple libraries For some reason python can not find the function implementation if it is in the cpp file. It happened to Joerg as well - is there anything else other than updating the include/Sconscript, ,src/Sconscript and pyext/misc.i ? > > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev