«

I am trying to find the best way of documenting (in code and
comments) ownership of secondary contained objects. This is my
current status, and I would appreciate feedback on it:



Case 1: When the secondary object is created with the object
and dies with the object.

Solution: Keep the secondary object as a stack variable, and
deallocation becomes a no-issue.

Example:

class Person
{
:
private:
Gender gender_;
};




Case 2: When a secondary object exists somewhere else (i.e. is owned
by someone else) but is referred to by the class.

Solution: Keep secondary object as a (possibly const) reference.
Pass reference through constructor or setter method.
The reference indicate that it exists somewhere else and the class
should never delete it.

Example:

class Person {
public:
Person(Person& mother);
private:
Person& mother_;
};



Case 3: Like case 2, but where the secondary object might be 0.

Solution: Keep secondary object as a (possibly const/const) pointer.
Pass pointer throgh constructor or setter method BUT DOCUMENT THE
FACT THAT OWNERSHIP IS ELSEWHERE.

Example:

class Person {
public:
void setSpouse(Person* spouse);
private:
Person* spouse_;
};



Case 4: A secondary object is created by a client and passed into
a class who becomes its owner.

Solution: Keep secondary object as a pointer internally and also
pass it as a pointer (even when it cannot be 0). Possibly document
the fact that the secondary object is now owned by the object, and
also document that it cannot be null by a precondition assert.

Example:

class Person {
public:
void setHairstyle(Hairstyle* hairstyle);
private:
Hairstyle* hairstyle_;
};


Person::~Person()
{
delete hairstyle_;
}



Currently I have trouble with case 3 where I need to document
(by comments) who owns the object, and case 4 where I pass an object
through a pointer even when it cannot be 0. I could technically pass
it as a reference but by passing it as a pointer I explicitly indicate
that it is *not* a reference to an object that exists elsewhere.
It then becomes (more) obvious where ownership is.

Any thoughts?

Thanks!

Hi

Jacob wrote:

> I am trying to find the best way of documenting (in code and
> comments) ownership of secondary contained objects. This is my
> current status, and I would appreciate feedback on it:
[... lots of raw pointers ...]
> Any thoughts?

You should consider using std::auto_ptr.
You might also have a look at the boost smart pointers
( http://www.boost.org/libs/smart_ptr/smart_ptr.htm ).

Markus

Jacob wrote:
> I am trying to find the best way of documenting (in code and
> comments) ownership of secondary contained objects. This is my
> current status, and I would appreciate feedback on it:

This is not hard. You simply comment the member variable as "owned" or
"held".

> Case 1: When the secondary object is created with the object
> and dies with the object.

[snip]
> Example:
>
> class Person
> {
> :
> private:
> Gender gender_;
> };

In this case, you don't even have to document that 'gender_' is "owned"
because it cannot be otherwise.

> Case 2: When a secondary object exists somewhere else (i.e. is owned
> by someone else) but is referred to by the class.

This is where things get interesting. When you give an external object
to a class to hold, always pass it as a pointer:

[snip]
> class Person {
> public:
> Person(Person& mother);
> private:
> Person& mother_;
> };

class Person {
Person *d_mother_p; // this person's mother (held)

public:
Person(Person *mother);
};

The main reason why you store 'mother_p' as a pointer (note the '_p'
suffix) is to denote that it is held (although it may be owned which is
why you need to document this fact). Since 'mother' is stored as a
pointer, you should pass it as a pointer both for reasons of symmetry,
and also to document to the reader of client code that the argument
might be held (but is definitely not copied).

For example, in the following snippet

Bar bar;
Foo foo(bar);
//...

it is not clear that 'bar' is held and not owned (via copying) by
'foo'. Hence, it is difficult to spot bugs involving the premature
deletion of the external object 'bar'. However,

Bar bar;
Foo foo(&bar);
//...

clearly hints at the likelyhood that 'bar' is meant to outlive 'foo'.

Your example is especially interesting because your constructor
parameterized by 'mother' is actually expressed as a copy constructor.
I don't know if this was deliberate, but it could certainly lead to
some interesting bugs.

> Case 3: Like case 2, but where the secondary object might be 0.

[snip]
> Example:
>
> class Person {
> public:
> void setSpouse(Person* spouse);
> private:
> Person* spouse_;
> };

Hapilly, this is the same as Case 2:

class Person {
Person *d_spouse_p; // this person's spouse (held)
public:
void setSpouse(Person *spouse);
// Set this person's spouse to the specified 'spouse'.
};

The fact that 'spouse' may be zero is an implementation detail that you
don't need to document in the interface. However, you might want to
externalize 'spouse_p' in which case you may add

const Person *spouse() const;
// Return this person's spouse, if one exists; otherwise
// return 0.

It's not important that 'spouse_p' can be null. It is important that
your accessor may *return* a null pointer.

> Case 4: A secondary object is created by a client and passed into
> a class who becomes its owner.

[snip]
> Example:
>
> class Person {
> public:
> void setHairstyle(Hairstyle* hairstyle);
> private:
> Hairstyle* hairstyle_;
> };
>
>
> Person::~Person()
> {
> delete hairstyle_;
> }

This case is also the same as Case 2:

class Person {
Hairstyle *d_hairstyle_p; // this person's hairstyle (owned)
public:
void setHairstyle(Hairstyle *hairstyle);
// Set this person's hairstyle to the specified
'hairstyle'.
};

However, this too is not a very good example. There are really two
cases where an external object may become "owned" by a 'Person'. In
both cases, the point of ownership relates more than anything to the
relative lifespan of the two objects in the caller.

The first case involves value-semantic objects (those which define copy
constructors and assignment operators). In this case, a 'Person' may
"own" an external object by copying its value:

class Person {
Hairstyle d_haristyle;
public:
void setHairstyle(const Hairstyle& hairstyle)
{ d_hairstyle = hairstyle; }
};

Passing by const reference is, by convention, an indication that the
argument may be copied by the other object, and the caller is free to
destroy the argument object at any point relative to the other object:

Foo foo;
{
Bar bar;
foo.setBar(bar);
}
foo.action(); // involves 'bar'

The second case involves relinquishing ownership of a dynamically
allocated resource to another object. However, in this case I would
suggest (as someone else did in this thread) the use of managed
pointers such as 'std::auto_ptr' or a shared pointer. However, be
careful to pass these by *modifiable* reference:

void setConnectionPool(std::auto_ptr<ConnectionPool>& pool);

The implementation of 'setConnectionPool' may take ownership of 'pool'
by making an assignment between 'auto_ptr's, or may chose to delegate
ownership of 'pool' to yet another object.

The usefulness of this technique becomes more apparent when you
allocate dynamic resources from an allocator other than new/delete. In
this case, you can bundle the object and the allocator in a managed
pointer so that the new owner does not need to be aware of the
particular allocator type used to allocate (and deallocate) the object.
(Sadly, 'auto_ptr' does not currently support this kind of interface.)

HTH, /david

wrote:

> The main reason why you store 'mother_p' as a pointer (note the '_p'
> suffix)

Yes I did note it, very ugly, why do you think that sticking that
abomination on the end is a good idea ?

Geo wrote:
> wrote:
>
> > The main reason why you store 'mother_p' as a pointer (note the '_p'
> > suffix)
>
> Yes I did note it, very ugly, why do you think that sticking that
> abomination on the end is a good idea ?

So I can tell which variables are pointers and which are not at a
glance. It's just a convention. Take it or leave it. /david

Jacob wrote:
> Markus Moll wrote:
>
>> You should consider using std::auto_ptr.
>> You might also have a look at the boost smart pointers
>> ( http://www.boost.org/libs/smart_ptr/smart_ptr.htm ).
>
>
> Though smart pointers may help you manage your
> memory, they will (I guess; I have limited experience
> in the field) fail to document ownership.
>
What do you mean by ownership? If you mean responsibility
for deallocating the object, smart pointers do that.
Everybody knows that's what smart pointers do. You can't
get more documented than that.

--
Joe Seigh

When you get lemons, you make lemonade.
When you get hardware, you make software.

wrote:

> So I can tell which variables are pointers and which are not at a
> glance. It's just a convention. Take it or leave it. /david

Leave it! It is a misconception to associate type with
modelling concepts. It's equivalent to:

int nTableInteger;
double temperatureDouble;

Only occationally I want to emphasize a name with a type;
like this:

string temperatureString = ...;
double temperature = temperatureString.toDouble();

Markus Moll wrote:

> You should consider using std::auto_ptr.
> You might also have a look at the boost smart pointers
> ( http://www.boost.org/libs/smart_ptr/smart_ptr.htm ).

Though smart pointers may help you manage your
memory, they will (I guess; I have limited experience
in the field) fail to document ownership.

It might be argued that it technically might not be
needed, but from a modelling point of view I
still find the raw counterparts valuable.

wrote:
>
> Jacob wrote:
>
>>I am trying to find the best way of documenting (in code and
>>comments) ownership of secondary contained objects. This is my
>>current status, and I would appreciate feedback on it:
>
>
> This is not hard. You simply comment the member variable as "owned" or
> "held".

I like to document as much as possible with the syntax
of the language. Relaying on comment conventions like this
is asking for trouble down the road.


>>Case 2: When a secondary object exists somewhere else (i.e. is owned
>>by someone else) but is referred to by the class.
>
>
> This is where things get interesting. When you give an external object
> to a class to hold, always pass it as a pointer:

I see no reason why this should not be a reference.
This is after all the classical situation where you
do use a reference, isn't it?


> Passing by const reference is, by convention, an indication that the
> argument may be copied by the other object, and the caller is free to
> destroy the argument object at any point relative to the other object:

You claim that parameters passed as const reference should
always be copied at the destination? That is not my current
practice, could you elaborate?

public:
void setMother(const Person& mother)
{
mother_ = mother;
}

private:
Person mother_;


I'd rather keep the private variable as a reference, but it is
of course then assumed that the object (the mother) outlive
the specific instance.

Jacob wrote:
> wrote:
>
> > So I can tell which variables are pointers and which are not at a
> > glance. It's just a convention. Take it or leave it. /david
>
> Leave it! It is a misconception to associate type with
> modelling concepts. It's equivalent to:
>
> int nTableInteger;
> double temperatureDouble;

I agree that associating a type with a concept (via a name) is not a
good idea, but that is not what I'm advocating. The '_p' convention
just says "this is a pointer", regardless of a particular type. While
it may seem redundant to you as the author, it is very useful to other
people who are reading the code.

In any case, this convention was...strongly suggested to me by my
current management, and I've grown to appreciate it enormously. YMMV.
/david