search

eggplantbren / DNest3

Diffusive Nested Sampling
GNU General Public License v3.0
20 stars 6 forks source link

add virtual destructor to base class/rename c++ headers #8

Closed beaylott closed 10 years ago

beaylott commented 10 years ago

This adds a virtual destructor to the abstract Model class and renames the C++ headers. These are the changes necessary for implementing a C/Python API based on call-backs (see: https://github.com/beaylott/DNest3 ) and it would be convenient if the main repository incorporated these changes ... and it maybe also is more conventional.

I guess is a matter of taste whether the default destructors should be specified in the interfaces or the object code... I have put them in the interfaces.

I have also updated all the examples with explicit default destructors so they are compatible with the change.

eggplantbren commented 10 years ago

I have no objection to these changes, except that they are a little disruptive to people already using DNest. Are you sure there isn't another way?

beaylott commented 10 years ago

I certainly dont want these changes accepted if it is going to cause you or the users of the repo inconvenience. And the header stuff is not important and is more aesthetics than anything else.

The addition of a virtual destructor to the abstract base class of the Model is also certainly interface breaking, however it is also just a matter of adding a dummy destructor to whatever has inherited from it. Using virtual destructors on abstract classes is recommended to avoid memory leaks and it makes doing clean up on the derived classes much easier.

eggplantbren commented 10 years ago

If the .h vs .hpp is only for aesthetics, I'm happy to make a change where Model has a virtual destructor. Nobody I know of is using pointers to Models in a polymorphic way, so it shouldn't affect anyone. Would this be okay?

beaylott commented 10 years ago

Yes that would be great thanks. That is really the main thing I need to hook in a Python interpreter (or at least hook it in in a clean way).

eggplantbren commented 10 years ago

Done!

On Wed, Dec 4, 2013 at 9:49 PM, Ben Aylott notifications@github.com wrote:

Yes that would be great thanks. That is really the main thing I need to hook in a Python interpreter (or at least hook it in in a clean way).

— Reply to this email directly or view it on GitHubhttps://github.com/eggplantbren/DNest3/pull/8#issuecomment-29788324 .

Dr Brendon J. Brewer Department of Statistics, The University of Auckland, New Zealand Ph: +64 27 500 1336 Web: http://stat.auckland.ac.nz/~brewer/