latest constructor changesets

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

latest constructor changesets

Rik-4
jwe,

One of the latest commit messages was "use default or deleted ctors/dtors/assignment ops in liboctave/util".  This is just about code readability, correct?  Replacing hand-rolled constructors with "= default" where there was in fact nothing different from what the compiler would automatically generate. There shouldn't be any performance changes over what we had before?

--Rik
Reply | Threaded
Open this post in threaded view
|

Re: latest constructor changesets

John W. Eaton
Administrator
On 9/10/19 11:40 AM, Rik wrote:

> One of the latest commit messages was "use default or deleted
> ctors/dtors/assignment ops in liboctave/util".  This is just about code
> readability, correct?  Replacing hand-rolled constructors with "=
> default" where there was in fact nothing different from what the
> compiler would automatically generate. There shouldn't be any
> performance changes over what we had before?

Yes, I've been making changes like that because I think it makes the
code easier to read and maintain.  I wouldn't expect any change in
performance.  I think it's best to explicitly state that the compiler
should generate these functions where possible.

As an example of the kind of error that can happen when writing them by
hand, I noticed one case in which the existing copy constructor and
assignment operator were not copying all class data members.  I couldn't
see any reason to omit any of them but it took some time to try to
determine whether the omission was intentional.  I eventually decided
that it was more likely that some data members were added but the copy
and assignment functions were not updated.  Having the compiler generate
these functions avoids that error and makes it easier to understand
immediately what the functions will do.  If they are written by hand,
you have to scan the list of data members that are copied in the
functions and compare with the class declaration to be sure that
everything is properly copied.

I'm also switching to using "= delete" declarations instead of declaring
copy constructors and assignment operators as private.  Using "= delete"
will always give a compile time error when a copy is requested instead
of a warning about "use of a private function in this context" or a
linker error if a copy is requested inside another member function.
Though we did try to mark these private declarations with a "No
copying!" comment, the "= delete" declaration leaves no room for
confusion about whether the function definitions were accidentally omitted.

jwe


Reply | Threaded
Open this post in threaded view
|

Re: latest constructor changesets

apjanke-floss


On 9/10/19 2:26 PM, John W. Eaton wrote:

> On 9/10/19 11:40 AM, Rik wrote:
>
>> One of the latest commit messages was "use default or deleted
>> ctors/dtors/assignment ops in liboctave/util".  This is just about
>> code readability, correct?  Replacing hand-rolled constructors with
>> "= default" where there was in fact nothing different from what the
>> compiler would automatically generate. There shouldn't be any
>> performance changes over what we had before?
>
> Yes, I've been making changes like that because I think it makes the
> code easier to read and maintain.  I wouldn't expect any change in
> performance.  I think it's best to explicitly state that the compiler
> should generate these functions where possible.
>
> As an example of the kind of error that can happen when writing them
> by hand, I noticed one case in which the existing copy constructor and
> assignment operator were not copying all class data members.  I
> couldn't see any reason to omit any of them but it took some time to
> try to determine whether the omission was intentional.  I eventually
> decided that it was more likely that some data members were added but
> the copy and assignment functions were not updated.  Having the
> compiler generate these functions avoids that error and makes it
> easier to understand immediately what the functions will do.  If they
> are written by hand, you have to scan the list of data members that
> are copied in the functions and compare with the class declaration to
> be sure that everything is properly copied.
>
> I'm also switching to using "= delete" declarations instead of
> declaring copy constructors and assignment operators as private. 
> Using "= delete" will always give a compile time error when a copy is
> requested instead of a warning about "use of a private function in
> this context" or a linker error if a copy is requested inside another
> member function. Though we did try to mark these private declarations
> with a "No copying!" comment, the "= delete" declaration leaves no
> room for confusion about whether the function definitions were
> accidentally omitted.
>
> jwe
>
>
This sounds better to me. As a relatively inexperienced C++ programmer,
these forms are more readily understandable to me.

Cheers,
Andrew