Re: [IMP-dev] [IMP-commits] r1297 - trunk/modules/algebra/include
It also means that when someone creates a default Transformation3D, you don't know if they want the identity transform or just will initialize it later. And it's not symmetric with the Vector3D and the Rotation3D.
On Jan 27, 2009, at 10:16 PM, Notification of IMP commits wrote:
> Author: kerenl@SALILAB.ORG > Date: 2009-01-27 22:16:23 -0800 (Tue, 27 Jan 2009) > New Revision: 1297 > > Modified: > trunk/modules/algebra/include/Transformation3D.h > Log: > change to default constrator to initilize the identity transformation > > Modified: trunk/modules/algebra/include/Transformation3D.h > =================================================================== > --- trunk/modules/algebra/include/Transformation3D.h 2009-01-27 > 22:26:43 UTC (rev 1296) > +++ trunk/modules/algebra/include/Transformation3D.h 2009-01-28 > 06:16:23 UTC (rev 1297) > @@ -25,6 +25,8 @@ > typedef Transformation3D This; > //! construct and invalid transformation > Transformation3D(){ > + trans_=Vector3D(0.0,0.0,0.0); > + rot_ = identity_rotation(); > } > /** basic constructor*/ > Transformation3D(const Rotation3D& r, const Vector3D& t) > > _______________________________________________ > IMP-commits mailing list > IMP-commits@salilab.org > https://salilab.org/mailman/listinfo/imp-commits
then maybe I should update that for Vector3D and Rotation3D as well, as it was not possible accessing Transformation3D which was constructed from the default one before. On Jan 28, 2009, at 7:44 AM, Daniel Russel wrote:
> It also means that when someone creates a default Transformation3D, > you don't know if they want the identity transform or just will > initialize it later. And it's not symmetric with the Vector3D and > the Rotation3D. > > > On Jan 27, 2009, at 10:16 PM, Notification of IMP commits wrote: > >> Author: kerenl@SALILAB.ORG >> Date: 2009-01-27 22:16:23 -0800 (Tue, 27 Jan 2009) >> New Revision: 1297 >> >> Modified: >> trunk/modules/algebra/include/Transformation3D.h >> Log: >> change to default constrator to initilize the identity transformation >> >> Modified: trunk/modules/algebra/include/Transformation3D.h >> =================================================================== >> --- trunk/modules/algebra/include/Transformation3D.h 2009-01-27 >> 22:26:43 UTC (rev 1296) >> +++ trunk/modules/algebra/include/Transformation3D.h 2009-01-28 >> 06:16:23 UTC (rev 1297) >> @@ -25,6 +25,8 @@ >> typedef Transformation3D This; >> //! construct and invalid transformation >> Transformation3D(){ >> + trans_=Vector3D(0.0,0.0,0.0); >> + rot_ = identity_rotation(); >> } >> /** basic constructor*/ >> Transformation3D(const Rotation3D& r, const Vector3D& t) >> >> _______________________________________________ >> IMP-commits mailing list >> IMP-commits@salilab.org >> https://salilab.org/mailman/listinfo/imp-commits > > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev
On Jan 28, 2009, at 7:48 AM, Keren Lasker wrote:
> then maybe I should update that for Vector3D and Rotation3D as well, > as it was not possible accessing Transformation3D which was > constructed from the default one before. I don't understand that sentence. But, if you are saying that Vector3D() should be Vector3D(0,0,0) I disagree and prefer it to be left undefined (or in a defined, but non-useful state). And thing Transformation3D should be changed to match.
sorry that my english is so bad daniel. I'll remove the change to the default const. as requested by both you and ben. On Jan 28, 2009, at 7:56 AM, Daniel Russel wrote:
> > On Jan 28, 2009, at 7:48 AM, Keren Lasker wrote: > >> then maybe I should update that for Vector3D and Rotation3D as >> well, as it was not possible accessing Transformation3D which was >> constructed from the default one before. > I don't understand that sentence. But, if you are saying that > Vector3D() should be Vector3D(0,0,0) I disagree and prefer it to be > left undefined (or in a defined, but non-useful state). And thing > Transformation3D should be changed to match. > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev
On Jan 28, 2009, at 7:59 AM, Keren Lasker wrote:
> sorry that my english is so bad daniel. My English would be pretty bad if I had been working at 3am this morning :-)
Daniel Russel wrote: > I don't understand that sentence. But, if you are saying that Vector3D() > should be Vector3D(0,0,0) I disagree and prefer it to be left undefined > (or in a defined, but non-useful state). And thing Transformation3D > should be changed to match.
Another way to think of things is consistency with primitive types. If in your code you say
int i; std::cout << i;
you will get some undefined output, since primitive types are not initialized (except for file-scope static variables, but we should probably avoid those anyway). For consistency
Vector3D i; std::cout << i;
should do something similar.
Ben
Daniel Russel wrote: > It also means that when someone creates a default Transformation3D, you > don't know if they want the identity transform or just will initialize > it later. And it's not symmetric with the Vector3D and the Rotation3D.
I agree - I would prefer default constructed objects to have invalid or at least undefined state. If you want the identity, you should explicitly initialize it that way - makes your code easier to follow.
Ben
participants (3)
-
Ben Webb -
Daniel Russel -
Keren Lasker