Destruct explicitly the pointer to the generator

[paolafer]

This PR adds an explicit call to the destructor of the generator. This way, the destructor of the generator class is called when the run manager is deleted.

[jacg]

Hold the generator by std::unique_ptr<T> rather than a raw pointer (T*), and all these things take care of themselves automatically.

Generally speaking, since C++11, anything that you own, should be held by std::unique_ptr<T>. I appreciate that Geant4 sometimes makes this difficult.

[paolafer]

Thanks, Jacek, I'll try the std::unique_ptr: I'm getting some errors, but I'll try to figure out how to use them.

[paolafer]

I'm afraid that in this particular case Geant4 won't allow us to use std::unique_ptr. The problem is that we use the same factory to create geometries, generators, persistency managers and actions and in the case of the actions the object the factory returns must be a raw pointer, because it has to be passed to the SetUserAction() function of the run manager, which doesn't accept std::unique_ptr. For simplicity, manteinance and homogeneity of the code, I think that it is better to keep the same factory for all.

[jacg]

It's fine for the factory to return a bare pointer, but whatever owns (is responsible for the lifetime of) the thing at the end of that pointer, should hold on to it via std::unique_ptr. In other words, you were planning to delete that pointer in the destructor of some class: that class should hold the thing via unique_ptr rather than a bare pointer, and then the destruction will happen automatically.

Specifically, (not tested, off the top of my head) rather than

class MyClass {
public:
  MyClass(T* t) : t{t} {}
  ~MyClass() { delete t; }
private:
  T* t;
};

you should have

class MyClass {
public:
  MyClass(T* t) : t{t} {}
private:
  std::unique_ptr<T> t;
};

or rather

class MyClass {
public:
  MyClass(std::unique_ptr<T> t) : t{std::move(t)} {}
private:
  std::unique_ptr<T> t;
};

to make it clear to the caller of MyClass' constructor, that it is going to take ownership.

In the first two cases, passing the T* to MyClass::MyClass looks no different from passing it to SetUserAction so there is no way to tell, by just reading the client code, who owns the T.

In the third case, the caller is obliged to pass in a std::unique_ptr<T> (trivially created from the bare pointer, right there) which sends a clear signal that MyClass takes ownership.

This is a huge improvement to the language that arrived in C++11. Take advantage of it!

[jacg]

It looks like you've got the hang of it now. Good work.

[paolafer]

I've cleaned up the history a little bit, let me know if there's anything else.