Investigating whether I need to use a auto_ptr

[Pep]

I have a method in a class like this

class myClass
{
... ctros, dtor, etc ...

string myClassMethod()
{
string myString = "";

... more code ....

myString = "new value";

return(myString);
}

};

So my question is, is this safe or will the returned string go missing
when execution goes out of scope, in which case I would be better of
passing back a string pointer in the form of a auto_ptr?

TIA,
Pep.

 

[Gianni Mariani]

Pep wrote:
....

string myClassMethod()
{ ....

So my question is, is this safe or will the returned string go missing
when execution goes out of scope, in which case I would be better of
passing back a string pointer in the form of a auto_ptr?

the std::string class is a container that can be copied. What you're
doing here is returning a copy of myString so there is no need to manage
the lifetime of the string.


On the other hand if you had written:

std::string & myClassMethod()

.... this would have been undefined.

 

[Pep]

Pep wrote:
...

the std::string class is a container that can be copied. What you're
doing here is returning a copy of myString so there is no need to manage
the lifetime of the string.


On the other hand if you had written:

std::string & myClassMethod()

... this would have been undefined.

So the rule I need to apply when checking the final production version
of my class library would be to ensure that all objects within the
library have a copy ctor?

Also, do not return references which I don't attempt as a rule?

Cheers,
Pep.

 

[peter koch]

Pep skrev:

Gianni Mariani wrote: [snip]

So the rule I need to apply when checking the final production version
of my class library would be to ensure that all objects within the
library have a copy ctor?

The first rule is: resource management (be it memory, handles or
anything else) should be automatic. In particular, releasing a resource
should happen in a destructor and nowhere else.

Also, do not return references which I don't attempt as a rule?

Never return references (this includes pointers) to local data. (that
was rule #2),

/Peter

 

[Pep]

Pep skrev:

Gianni Mariani wrote: [snip]

So the rule I need to apply when checking the final production version
of my class library would be to ensure that all objects within the
library have a copy ctor?

The first rule is: resource management (be it memory, handles or
anything else) should be automatic. In particular, releasing a resource
should happen in a destructor and nowhere else.

Also, do not return references which I don't attempt as a rule?

Never return references (this includes pointers) to local data. (that
was rule #2),

/Peter

Thanks very much.

Can I ask you one last question regarding maps?

Is it safe to return a pointer to the data portion of a map that is
held in a object during the life cycle of the object.

i.e.

If I have a object that contains a map, can I return pointers such as

dataField* dataObject::getField(const string fieldName)
{
dataField* field = new dataField();
map_of_fields::iterator itor = fields.find(fieldName);

if (itor != fields.end())
{
field = (*itor).second;
}

return(field);
}

I am about to turn the field pointer in to a auto_ptr to ensure it is
resource safe but I am unsure how safe the map pointers would be.

The intention would be that the pointers would only be used while the
dataObject is in scope and discarded when the dataObject is destroyed.
Essentially dataObject is just a container of data for my program.

Cheers,
Pep.

 

[mlimber]

Never return references (this includes pointers) to local data.

"Never" rules like this are generally inaccurate (cf.
http://www.parashift.com/c++-faq-lite/basics-of-inheritance.html#faq-19.8).
std::vectors return references, as do some singleton implementations,
etc. There are any number of other circumstances where returning a
reference is not only not bad but also preferable.

Cheers! --M

 

[mlimber]

Can I ask you one last question regarding maps?

Is it safe to return a pointer to the data portion of a map that is
held in a object during the life cycle of the object.

It depends.

i.e.

If I have a object that contains a map, can I return pointers such as

dataField* dataObject::getField(const string fieldName)
{
dataField* field = new dataField();
map_of_fields::iterator itor = fields.find(fieldName);

if (itor != fields.end())
{
field = (*itor).second;

First, if this statement executes, your new'ed dataField above will
leak. Second, assuming your map_of_fields is of type
std::map<std::string, dataField*>, you are playing with fire. You don't
get automatic cleanup or exception safety here with the dataFields. Use
a smart pointer (or another RAII mechanism) instead of a raw pointer
(cf. http://www.artima.com/cppsource/bigtwo.html).

}

return(field);
}

I am about to turn the field pointer in to a auto_ptr to ensure it is
resource safe but I am unsure how safe the map pointers would be.

But do you want it to delete the value in the map, if one is found?
(Answer: almost certainly not.) That is what using an auto_ptr would do
here.

The intention would be that the pointers would only be used while the
dataObject is in scope and discarded when the dataObject is destroyed.
Essentially dataObject is just a container of data for my program.

Proper RAII would mean that your dataObject will manage the lifetime of
all its members or at the very least, transfer (or share) that
responsibility by returning an auto_ptr (or another smart pointer). In
that case, you could return a pointer to a member safely, but it might
be better to return a reference and throw an exception if the key is
not found.

Cheers! --M

 

[peter koch]

mlimber skrev:

"Never" rules like this are generally inaccurate (cf.
http://www.parashift.com/c++-faq-lite/basics-of-inheritance.html#faq-19.8).
std::vectors return references, as do some singleton implementations,
etc. There are any number of other circumstances where returning a
reference is not only not bad but also preferable.

Cheers! --M

I could not agree more! My wording was inaccurate - as I meant you
should NEVER return references to automatic ("stack local") data.

/Peter

 

[peter koch]

Pep skrev:

Pep skrev:

[snip]
Thanks very much.

Can I ask you one last question regarding maps?

Is it safe to return a pointer to the data portion of a map that is
held in a object during the life cycle of the object.

It is just fine to return references to members of an object. Just
remember that the reference in question is invalidated as soon as the
object is destroyed. But your code is bad.

i.e.

If I have a object that contains a map, can I return pointers such as

dataField* dataObject::getField(const string fieldName)
{
dataField* field = new dataField();

Here you new an object.

map_of_fields::iterator itor = fields.find(fieldName);

if (itor != fields.end())
{
field = (*itor).second;

And here you overwrite the object.

}

return(field);
}

I am about to turn the field pointer in to a auto_ptr to ensure it is
resource safe but I am unsure how safe the map pointers would be.

But when the field is in the map, you would not want to delete the
field: in that case you'll end up with a deleted object in your map.
The solution would be to return 0 when you do not find the element:

dataField* dataObject::getField(const string fieldName)
{
map_of_fields::iterator itor = fields.find(fieldName);

if (itor != fields.end())
{
return (*itor).second;
}

return(0);
// imo would be better to write:
// return itor == fields.end() = 0: itor->second;
}

 

[Pep]

Pep skrev:

Pep skrev:

[snip]
Thanks very much.

Can I ask you one last question regarding maps?

Is it safe to return a pointer to the data portion of a map that is
held in a object during the life cycle of the object.

It is just fine to return references to members of an object. Just
remember that the reference in question is invalidated as soon as the
object is destroyed. But your code is bad.

i.e.

If I have a object that contains a map, can I return pointers such as

dataField* dataObject::getField(const string fieldName)
{
dataField* field = new dataField();

Here you new an object.

map_of_fields::iterator itor = fields.find(fieldName);

if (itor != fields.end())
{
field = (*itor).second;

And here you overwrite the object.

}

return(field);
}

I am about to turn the field pointer in to a auto_ptr to ensure it is
resource safe but I am unsure how safe the map pointers would be.

But when the field is in the map, you would not want to delete the
field: in that case you'll end up with a deleted object in your map.
The solution would be to return 0 when you do not find the element:

dataField* dataObject::getField(const string fieldName)
{
map_of_fields::iterator itor = fields.find(fieldName);

if (itor != fields.end())
{
return (*itor).second;
}

return(0);
// imo would be better to write:
// return itor == fields.end() = 0: itor->second;
}

The intention would be that the pointers would only be used while the
dataObject is in scope and discarded when the dataObject is destroyed.
Essentially dataObject is just a container of data for my program.

Cheers,
Pep.

Sorry but I did not give a complete working example for brevity.

The new'd object has a type() method which for a default ctor returns
undefined so that the client code can continue with known behaviour. In
the event of a undefined object being returned, the client will
continue to populate in a manner different to how it would handle a
defined object which can be of varying types. I do this simply because
it saves on programming logic where I would have to check for a null
return and then create a new object in various places that the method
will be used.

Cheers,
Pep.

 

[Pep]

It depends.


First, if this statement executes, your new'ed dataField above will
leak. Second, assuming your map_of_fields is of type
std::map<std::string, dataField*>, you are playing with fire. You don't
get automatic cleanup or exception safety here with the dataFields. Use
a smart pointer (or another RAII mechanism) instead of a raw pointer
(cf. http://www.artima.com/cppsource/bigtwo.html).


But do you want it to delete the value in the map, if one is found?
(Answer: almost certainly not.) That is what using an auto_ptr would do
here.


Proper RAII would mean that your dataObject will manage the lifetime of
all its members or at the very least, transfer (or share) that
responsibility by returning an auto_ptr (or another smart pointer). In
that case, you could return a pointer to a member safely, but it might
be better to return a reference and throw an exception if the key is
not found.

Cheers! --M

Trying to get the use of auto_ptr's is starting to confuse me.

Firstly you are right in your assumption that the map is of the type
std::map<std::string, dataField*>. The data element is created as a
new'd object upon insertion.

I am trying to build a object that stores the different types of
dataField objects in the map and then allows them to be retrieved by
key at different points in my application without disturbing the map
but still allowing for the dataField object to be modified.

So having started to investigate the usage of auto_ptr's I was
beginning to think it best that the returned dataField ptr be a
auto_ptr but I can see your obvious point that the the returned auto
pointer would delete the dataField in the map.

Bringing me back to my original design that simply returning dataField
would be safe given that it is stored in the map. However, my confusion
of auto_ptr usage lead me to believe that my original design is prone
to leakage.

So would it be better to do this

dataField* dataObject::getField(const string fieldName)
{
map_of_fields::iterator itor = fields.find(fieldName);

return((itor == fields.end()) ? new dataField() :
(*itor).second);
}

The object that stores the map is in scope for the complete lifetime of
the application and thus should preserve the persistence of the stored
objects. The point in sending back a new'd dataField is that the
default ctor sets the dataField type to null which the various clients
of the method expect and handle accordingly.

Cheers,
Pep.

 

[Clark S. Cox III]

dataField* dataObject::getField(const string fieldName)
{
map_of_fields::iterator itor = fields.find(fieldName);

return((itor == fields.end()) ? new dataField() :
(*itor).second);
}

The object that stores the map is in scope for the complete lifetime of
the application and thus should preserve the persistence of the stored
objects. The point in sending back a new'd dataField is that the
default ctor sets the dataField type to null which the various clients
of the method expect and handle accordingly

Why not use use a NULL pointer for that "null" dataField type?

 

[Pep]

Why not use use a NULL pointer for that "null" dataField type?

The clients that use the method do something like this

switch (myDataObject.getField("field name")->getType())
{

case fieldType::Type1
{
.. some code ..
}
case fieldType::Type2
{
.. some code ..
}
case fieldType::Type3
{
.. some code ..
}
default:
{
.. some code ..
}

}

but if I use a null pointer then I need to wrap the switch statement
with a

if (myDataObject.getField("field name") != null)
{

.. switch statement ..

}
else
{
}

Which is not as nice as it would be with a null field type.

So the question I need to be sure about, is if I return a new'd
dataField will it leak?

Cheers,
Pep.

 

[eriwik]

switch (myDataObject.getField("field name")->getType())
{

case fieldType::Type1
{
.. some code ..
}
case fieldType::Type2
{
.. some code ..
}
case fieldType::Type3
{
.. some code ..
}
default:
{
.. some code ..
}

}but if I use a null pointer then I need to wrap the switch statement
with a

if (myDataObject.getField("field name") != null)
{

.. switch statement ..

}else
{

}Which is not as nice as it would be with a null field type.

So the question I need to be sure about, is if I return a new'd
dataField will it leak?

Only if you don't delete it when you are done with it, if the method
calling getField() deletes the returned object if it's type is null
then it would be fine. Except that this would prevent you from storing
a field with a null-type in the map, since it would be deleted when
returned from getField().

I was about to suggest that you return a reference to the object in the
collection instead of a pointer, but that does not work very well if
you have to return a new object if none is found in the map. In other
cases, where you can be sure that the object is in the map, a reference
is a good alternative to a pointer, it does not require copying of the
object but it can't be deleted either.

 

[Pep]

Only if you don't delete it when you are done with it, if the method
calling getField() deletes the returned object if it's type is null
then it would be fine. Except that this would prevent you from storing
a field with a null-type in the map, since it would be deleted when
returned from getField().

I was about to suggest that you return a reference to the object in the
collection instead of a pointer, but that does not work very well if
you have to return a new object if none is found in the map. In other
cases, where you can be sure that the object is in the map, a reference
is a good alternative to a pointer, it does not require copying of the
object but it can't be deleted either.

Good points. In fact that is the problem that I started looking at the
use of auto_ptr for. The fact that I wanted to be able to use the
getType() method of the returned dataField in a switch statement, which
unfortunately does cause leakage because the switch statement leaves
the dataField out of scope at the end and as I do not instantiate the
object before the switch means that I cannot destroy it.

I think that no matter how much I would like to use this in the manner
described, I am going to have to bite the bullet and return a null
instead of my own null type.

Mostly I look for elegant code where I can but in this instance the
elegance I am striving for is causing logical problems.

Thanks everyone for the advice and help, I will change the code and get
on with the more pressing issues with the project.

Cheers,
Pep.