std::map question

[Flzw]

Well I have a map like this :

std::map <string, CObject> ObjectList;

I have a function like this :

CObject* NewObject( char* Name, CArg* Arg)
{
std::string key = Name;
ObjectList[ key] = CObject( Name, Arg);
return &ObjectList[Name];
}

I tried to compile and it complained that there was no default constructor
for CObject

So I changed my constructor declaration to :
CObject::CObject( char* Name = "Default", CArg* Arg = NULL);

Just to sse because I can't create it without a valid CArg pointer.

it compiles when I call NewObject it crashes, with the debugger I saw it
seemed to call the constructor with the correct arguments first, and then
call it again with no arguments (which in this case makes my program crash),
but I don't understant why.

If someon ecould explain what I don't understand here....

An other quick question, does &ObjectList[ Name] return NULL if Name does
not match a key in the map ?

Thanks in advanec.

 

[Victor Bazarov]

Well I have a map like this :

std::map <string, CObject> ObjectList;

I have a function like this :

CObject* NewObject( char* Name, CArg* Arg)
{
std::string key = Name;
ObjectList[ key] = CObject( Name, Arg);
return &ObjectList[Name];
}

I tried to compile and it complained that there was no default constructor
for CObject

There is no requirement for the contained objects to have the default
c-tor _unless_ you're using it yourself. They need to be _assignable_
and _copy-constructible_. If you used

ObjectList.insert(make_pair(key, CObject(Name,Arg)));

instead of

ObjectList[key] = CObject(Name, Arg);

you wouldn't need it default-constructible.

So I changed my constructor declaration to :
CObject::CObject( char* Name = "Default", CArg* Arg = NULL);

Well, that's a very bad declaration. It seems to suggest taht you are
using dynamic memory in your CObject, which means you probably screw up
the dynamic memory management. Like, it's deleted twice (usually happens
when you don't have the copy-c-tor correctly implemented). Read about
"the Rule of Three" and think about the _ownership_ of the dynamic memory
allocated in your objects.

The badness of that declaration is in the fact that you pass those chars
and CArgs by a pointer to non-const object, which probably means that
instead of creating the CObject's own copies of those, you just copy the
pointer values there. Well, hopefully not. Just making sure...

Just to sse because I can't create it without a valid CArg pointer.

Again, you really don't need the default c-tor here.

it compiles when I call NewObject it crashes, with the debugger I saw it
seemed to call the constructor with the correct arguments first, and then
call it again with no arguments (which in this case makes my program crash),
but I don't understant why.

Because the expression

ObjectList[key] = CObject(blah);

needs to (a) create an empty CObject, (b) create a CObject with 'blah'
parameters and then (c) call the assignment from the latter to the former.

If someon ecould explain what I don't understand here....

You don't understand several things. Get a good book and read about
standard containers, their behaviour, and the requirements they place
on the contained classes. Also get a good book and read about dynamic
memory management.

An other quick question, does &ObjectList[ Name] return NULL if Name does
not match a key in the map ?

No. It inserts a new object and returns a reference to it. RTFM.

Victor

 

[David Hilsee]

Well I have a map like this :

std::map <string, CObject> ObjectList;

I have a function like this :

CObject* NewObject( char* Name, CArg* Arg)
{
std::string key = Name;
ObjectList[ key] = CObject( Name, Arg);
return &ObjectList[Name];
}

I tried to compile and it complained that there was no default constructor
for CObject

So I changed my constructor declaration to :
CObject::CObject( char* Name = "Default", CArg* Arg = NULL);

Just to sse because I can't create it without a valid CArg pointer.

it compiles when I call NewObject it crashes, with the debugger I saw it
seemed to call the constructor with the correct arguments first, and then
call it again with no arguments (which in this case makes my program crash),
but I don't understant why.

If someon ecould explain what I don't understand here....

I don't understand the behavior you described, but I'll explain what the
behavior should be.

ObjectList[key] = CObject(Name, Arg);

The line above creates a default-constructed CObject and attempts to insert
it into the std::map as the value mapped to the key. If there is already a
value mapped to the key, then the std::map is left unchanged. The
std::map:perator[] then returns a reference to the CObject instance mapped
to key (could be the default-constructed object I just mentioned or the one
that was already present). Then, CObject's assignment operator is invoked,
effectively replacing the contents of the instance stored inside the
std::map.

If you don't want to have to specify a default constructor, you can use
std::map::insert instead of operator[]. That function returns an iterator
and a boolean, so you can check to see if the insert did not insert a new
key-value pair into the map and then use the iterator to modify the existing
value in that case.

An other quick question, does &ObjectList[ Name] return NULL if Name does
not match a key in the map ?

No, in that case, operator[] inserts a new key-value pair into the map and
you get a pointer to the default-constructed CObject instance.

 

[David Hilsee]

to key (could be the default-constructed object I just mentioned or the one
that was already present). Then, CObject's assignment operator is

invoked,

That should read "could be a copy of the".

No, in that case, operator[] inserts a new key-value pair into the map and
you get a pointer to the default-constructed CObject instance.

Again, that should read "to a copy of the".

 

[Flzw]

Alright, I understood my mistakes.

So, how could I get a pointer to an object in the map if I have the
corresponding key without using the [] operator ?
and without creating a new object if the key doesn't already exist.

Thanks.

 

[David Hilsee]

Alright, I understood my mistakes.

So, how could I get a pointer to an object in the map if I have the
corresponding key without using the [] operator ?
and without creating a new object if the key doesn't already exist.

I think you're looking for the find() member function. It returns an
iterator (end() if the key was not found).

 

[Flzw]

If you used

ObjectList.insert(make_pair(key, CObject(Name,Arg)));

This doesn't seem to work, the object is constructed and then destructed
right away.

 

[David Hilsee]

x

This doesn't seem to work, the object is constructed and then destructed
right away.

It sounds like you don't understand what's going on. Of course the object
is constructed and then destructed; it's a temporary object. It is
temporarily constructed, passed to make_pair, copied into an instance of
some std:air instantiation, which is then passed to insert, where another
copy of the object is made to hold it inside the std::map. There may be
another copy operation that I missed, but that's beside the point. The
point is that there are at least 3 instances of CObject involved in the
above code. If you're experiencing odd behavior at runtime, it's possible
that you did not properly implement the assignment operator and copy
constructor for your CObject class.

 

[Flzw]

It sounds like you don't understand what's going on. Of course the object
is constructed and then destructed; it's a temporary object. It is
temporarily constructed, passed to make_pair, copied into an instance of
some std:air instantiation, which is then passed to insert, where another
copy of the object is made to hold it inside the std::map.

Yes I understood this after I posted but the class constructor is called
once and the destructor is called three times, I undertand 3 objects are
made via copy operation.

My problem is that in my class CObject there is an Handle to an OS related
object which is constructed in the CObject constructor and destroyed in the
CObject destructor.
So in this case it is destroyed when the first temp CObject is destroyed and
the object in the map does not have a valid handle.

How could I work around this ? Is there a way to insert an Object without
involving temp CObject ? (which would also be much more efficient I
believe...)

 

[David Hilsee]

Yes I understood this after I posted but the class constructor is called
once and the destructor is called three times, I undertand 3 objects are
made via copy operation.

My problem is that in my class CObject there is an Handle to an OS related
object which is constructed in the CObject constructor and destroyed in the
CObject destructor.
So in this case it is destroyed when the first temp CObject is destroyed and
the object in the map does not have a valid handle.

How could I work around this ? Is there a way to insert an Object without
involving temp CObject ? (which would also be much more efficient I
believe...)

You could use reference counting. That way, multiple instances of CObject
may refer to the same handle and the OS will only be destroyed when the last
CObject that refers to it is destroyed. There is are some examples of this
in the FAQ (http://www.parashift.com/c++-faq-lite/) in section 16
("http://www.parashift.com/c++-faq-lite/freestore-mgmt.html"). You could
also instead store pointers to dynamically allocated instances of CObject so
they aren't copied when you update your std::map. If you use pointers, then
I would recommend using a "smart" pointer class (e.g. boost:shared_ptr) to
make them easier to deal with. Of course, that would just be reference
counting in a slightly different form.

 

[Kai-Uwe Bux]

My problem is that in my class CObject there is an Handle to an OS related
object which is constructed in the CObject constructor and destroyed in
the CObject destructor.
So in this case it is destroyed when the first temp CObject is destroyed
and the object in the map does not have a valid handle.

How could I work around this ? Is there a way to insert an Object without
involving temp CObject ? (which would also be much more efficient I
believe...)

It would be better not to *work around* this but to redesign the
implementation of CObject. The way it is right now, destruction of one
instance will invalidate all copies that exist somewhere else. This does
not only make the use of containers impossible but is somewhat dangerous
anyway.

One possible fix is to use a shared pointer to the OS resource instead of
an ordinary pointer. These pointers keep a reference count an destroy the
object refered to when the last pointer dies. So, wrap the OS resource into
a small class where the constructor acquires the resource and the destuctor
releases it; and use a shared pointer in CObject to point to a wraper
object.


Best

Kai-Uwe

 

[Flzw]

It would be better not to *work around* this but to redesign the

implementation of CObject. The way it is right now, destruction of one
instance will invalidate all copies that exist somewhere else. This does
not only make the use of containers impossible but is somewhat dangerous
anyway.

True.

But I thought std containers were efficient. Here with this line, it creates
4 instances of the object and deletes three... Is it really worth it ?

I mean, I will switch to std::map <string, CObject*> from std::map <string,
CObject> and handle new / delete myself I won't need to introduce shared
pointers and it will be MUCH faster (to code and to run).

 

[Kai-Uwe Bux]

True.

But I thought std containers were efficient. Here with this line, it
creates 4 instances of the object and deletes three... Is it really worth
it ?

Very often, the compiler can optimize some of these away. So I would
consider performance a minor problem unless the CObject class is *really*
costly to copy.

I mean, I will switch to std::map <string, CObject*> from std::map
<string, CObject> and handle new / delete myself I won't need to introduce
shared pointers and it will be MUCH faster (to code and to run).

That is another possibility. The problems with using raw pointers are
mostly in maintaining code: new/delete are global operations an humans
usually have only local vision when editing programms. Whether this
solution is good for your application depends entirely on the overall
designs of your programm and not so much on the part that has been under
discussion in this thread.


Best

Kai-Uwe Bux

 

[Jonathan Mcdougall]

Well I have a map like this :

std::map <string, CObject> ObjectList;

This will insert copies of strings and copies of CObject. std::string can
manage itself and is usually implemented using ref counting, so that's okay.
The thing is, it will also insert copies of CObjects. If CObject is not
implemented in a way that
1) copy construction is efficient and
2) copies are equivalent
the STL containers won't be of much use.

If copy construction is not efficient, you will have problems since at four
copies can be made by the various functions. For example,

map.insert(std::make_pair(key, value));

calls these (at least)

make_pair() takes key and value by value
pair's ctor makes copies of them
pair's cpy-ctor copies key and value
map::insert() copies key and value in container

So having an efficient copy-ctor is almost mandatory. Since all these
copies are made, if two copies are not equivalent, the object cannot be used
in containers. That is the case of your CObject class.

I have a function like this :

CObject* NewObject( char* Name, CArg* Arg)
{
std::string key = Name;
ObjectList[ key] = CObject( Name, Arg);

This line is very important. It first constructs a CObject (on the right
side) and then calls operator[] for ObjectList on the left side. This
operator returns a reference to the value having that key, but creates that
value if the key is not found. The only way it can create a new value is to
use the default constructor, which seems to be disabled in your class.

This is not the recommended way of inserting values in a std::map since it
is less efficient and can cause errors. You must first check if the key
exists. If it does, don't insert it, and if it doesn't, insert it :

// you should always typedef STL containers
typedef std::map <string, CObject> ObjectList;
ObjectList ol;

ObjectList::iterator itor = ol.find(key);
if ( itor == ol.end() )
{
// the key does not exist
ol.insert(std::make_pair(key, value));
}
else
{
// the key exists
}

return &ObjectList[Name];

return &(itor->second);

}

I tried to compile and it complained that there was no default constructor
for CObject

So I changed my constructor declaration to :
CObject::CObject( char* Name = "Default", CArg* Arg = NULL);

This kind of error is not generated because of the argument's values, but
because of the argument count. If the compiler complains that there is
default ctor available, its because somewhere you're calling the default
constructor, that is, you construct an object with no arguments.

Just to sse because I can't create it without a valid CArg pointer.

That would be a runtime error, not a compile-time error.

it compiles

What? If your compiler accepts code that calls functions with the wrong
agument count, change the compiler.


Now, for the solution, you can do many things. The two more obvious would
be
1) put pointers in the map
2) change CObject's copy constructor and destructor

By putting pointers in the map, you solve all the problems related to
copying and destroying, at the cost of runtime speed, since you must
allocate the objects on the heap. There are some memory-related potential
problems, since _you_ are responsible for deleting the objects. To be sure,
you should use some kind of reference-counted pointers (a simple
hand-crafted one could work, but check out the boost library for
shared_ptr).

But I think the real solution would be to implement the reference counting
system in the CObject class itself, so that copies are equivalent. That
way, with

CObject o;
CObject p;

p = o;

'p' should be equivalent to 'o'. This implies two things : that p and o
must work the same way after being copied and that if one of them is
destroyed, the other can continue to work the same way. By using reference
counting, if p is destroyed before o for example, you can prevent p from
releasing resources if o needs them. That's the basics of reference
counting.


Jonathan

 

[David Hilsee]

True.

But I thought std containers were efficient. Here with this line, it creates
4 instances of the object and deletes three... Is it really worth it ?

I mean, I will switch to std::map <string, CObject*> from std::map <string,
CObject> and handle new / delete myself I won't need to introduce shared
pointers and it will be MUCH faster (to code and to run).

If you are allocating memory dyanmically and it is possible for exceptions
to be thrown, it can be difficult to guarantee leak-free code unless there
is an object that is responsible for managing the memory. If you take the
"easy" route, then you will probably write code that can leak memory in
certain cases. Also, reference counted pointers can provide other benefits,
because they can alleviate the need to explicitly manage the lifetime of an
object, making the code simpler and more likely to be correct. I doubt that
your predictions about the improvement in the efficiency of the code will be
realized, especially when you consider that the string is probably managing
a dynamically allocated buffer and the object is managing a handle to an OS
resource. The overhead of the smart pointer will most likely be negligible
when compared to the rest of the code. Profile it and see if it's something
to be concerned about.