Created an attachment (id=104) [details]
automate memory management of RandomVariable

It would be possible to Ptr<RandomVariable> too but I decided against this
because it would have broken the user-visible API where you can pass around a
UniformVariable class by value to classes which need a RandomVariable.

(In reply to comment #1)
> Created an attachment (id=104) [edit] [details]
> automate memory management of RandomVariable
> 

Mathieu, could you provide a summary of this patch?  From what I see, it
appears that RandomVariables now are passed only by value.  It also appears
that there is some renaming and some extra indirection introduced into the
object and ownership heirarchy (RandomVariable owns a RandomVariableBase, which
is subclassed by Impl classes?)

(In reply to comment #3)

> Mathieu, could you provide a summary of this patch?  From what I see, it
> appears that RandomVariables now are passed only by value.  It also appears

yes.

> that there is some renaming and some extra indirection introduced into the
> object and ownership heirarchy (RandomVariable owns a RandomVariableBase, which
> is subclassed by Impl classes?)

Yes. Details below.

The previously-existing RandomVariable base class has been renamed to
RandomVariableBase. The previously-existing XXXVariable classes have been
renamed to XXXVariableImpl and they now subclass RandomVariableBase. Finally, a
new RandomVariable base class has been introduced which holds a pointer to a
RandomVariableBase subclass and does the memory management of the underlying
RandomVariableBase subclass. Basically, the RandomVariable class is not a smart
pointer which knows how to copy and delete RandomVariableBase subclasses. The
other XXXRandomVariable subclasses of the RandomVariable base class are just
conveniant constructors and were added to avoid changing the user-visible API.
i.e., to make it still possible to pass a temporary UniformVariable through a
function call.

This patchset is basically replacing the existing C++ variables of type
"RandomVariable *" into variables of type "RandomVariable". i.e., it is
removing the pointer and relieves the user from doing memory management
himself. As I said earlier, it would be trivially possible to achieve the same
result by using Ptr<RandomVariable> but this would break the user-visible API
since the user would not be able anymore to pass around temporary
UniformVariable objects.

(In reply to comment #4)
> RandomVariableBase subclass. Basically, the RandomVariable class is not a smart

This should have read:

Basically, the RandomVariable class is _now_ a smart point [...]

Just to be clear on declaration semantics, I now do the following to get
different distributions:

RandomVariable x = UniformVariableImpl(); 
RandomVariable y(ParetoVariableImpl());
//etc

Correct?  My only issue then is the fact that user code may have to include
classes with the word "Impl" in the name which is scary.  Perhaps the
RandomVariableBase subclasses could leave off the Impl naming?

Other than this clarification, I wouldn't be opposed to having this merged for
this month's release.

(In reply to comment #6)
> Just to be clear on declaration semantics, I now do the following to get
> different distributions:
> 
> RandomVariable x = UniformVariableImpl(); 
> RandomVariable y(ParetoVariableImpl());
> //etc
> 
> Correct?  My only issue then is the fact that user code may have to include

nope :)

> classes with the word "Impl" in the name which is scary.  Perhaps the
> RandomVariableBase subclasses could leave off the Impl naming?

The user code stays the same and does this:

RandomVariable x = UniformVariable ();

The UniformVariable class constructor calls RandomVariable (new
UniformVariableImpl ()) but this is done behind the scene on behalf of the
user.

> Other than this clarification, I wouldn't be opposed to having this merged for
> this month's release.

What you suggest could be used to achieve the same syntax I think but I have
not tried to implement it. Do you see this as a more desirable solution and do
you want me to attempt to implement it ?

(In reply to comment #7)
[snip]
> What you suggest could be used to achieve the same syntax I think but I have
> not tried to implement it. Do you see this as a more desirable solution and do
> you want me to attempt to implement it ?

No, I am fine with what you've posted.

changeset:   28ce210b91bb