learner's question on populating vector< pair<int, string>* > asmember

[subramanian100in]

Consider a class that has
vector< pair<int, string>* > c;
as member data object.

I need to use operator>> to store values into this container object
and
operator<< to print the contents of the container.

I have written both these operators as non-friend functions.

The destructor deletes all the pair<int, string> pointers in the
vector.

My doubt is that I am using a store_pair() function which allocates
memory
for each pair<int, string> pointer and stores into the container; but
the
dtor deletes these pointers thereby releasing the memory.

Kindly let me know if the approach in this program is correct.

Here is the full program:

#include <cstdlib>
#include <iostream>
#include <vector>
#include <utility>

using namespace std;

class Test
{
public:
Test();
~Test();

typedef pair<int, string> pair_type;
typedef vector<pair_type*> container_type;

void store_pair(const pair_type& arg);
const container_type& container() const { return c; }

private:
container_type c;
};

Test::Test() : c()
{
}

void Test::store_pair(const pair_type& arg)
{
pair_type* p = new pair_type(arg.first, arg.second);
c.push_back(p);
}

Test::~Test()
{
for (container_type::iterator it = c.begin(); it != c.end(); +
+it)
{
delete *it;
*it = 0;
}
}

istream& operator>>(istream& is, Test& obj)
{
while (is)
{
int x;
string str;
is >> x >> str;

if (is)
{
Test:air_type temp(x, str);
obj.store_pair(temp);
}
}

return is;
}

ostream& operator<<(ostream& os, const Test& obj)
{
for (Test::container_type::const_iterator it =
obj.container().begin();
it != obj.container().end();
++it)
os << (**it).first << " " << (*it)->second << endl;

return os;
}

int main()
{
Test obj;

cin >> obj;

cout << obj;

return EXIT_SUCCESS;
}

Thanks
V.Subramanian

 

[osama178]

Consider a class that has
vector< pair<int, string>* >  c;
as member data object.

I need to use operator>> to store values into this container object
and
operator<< to print the contents of the container.

I have written both these operators as non-friend functions.

The destructor deletes all the pair<int, string> pointers in the
vector.

My doubt is that I am using a store_pair() function which allocates
memory
for each pair<int, string> pointer and stores into the container; but
the
dtor deletes these pointers thereby releasing the memory.

Kindly let me know if the approach in this program is correct.

Here is the full program:

#include <cstdlib>
#include <iostream>
#include <vector>
#include <utility>

using namespace std;

class Test
{
public:
Test();
~Test();

typedef pair<int, string> pair_type;
typedef vector<pair_type*> container_type;

void store_pair(const pair_type& arg);
const container_type& container() const { return c; }

private:
container_type c;

};

Test::Test() : c()
{

}

void Test::store_pair(const pair_type& arg)
{
        pair_type* p = new pair_type(arg.first, arg.second);
        c.push_back(p);

}

Test::~Test()
{
        for (container_type::iterator it = c.begin(); it != c.end(); +
+it)
        {
                delete *it;
                *it = 0;
        }

}

istream& operator>>(istream& is, Test& obj)
{
        while (is)
        {
                int x;
                string str;
                is >> x >> str;

                if (is)
                {
                        Test:air_type temp(x, str);
                        obj.store_pair(temp);
                }
        }

        return is;

}

ostream& operator<<(ostream& os, const Test& obj)
{
        for (Test::container_type::const_iterator it =
obj.container().begin();
              it != obj.container().end();
              ++it)
                os << (**it).first << "   " << (*it)->second << endl;

        return os;

}

int main()
{
        Test obj;

        cin >> obj;

        cout << obj;

        return EXIT_SUCCESS;

}

Thanks
V.Subramanian

#include<string> in your program and it should work fine.

 

[Ian Collins]

#include<string> in your program and it should work fine.

Did you have to quote the entire message just to add that?

 

[Ian Collins]

Consider a class that has
vector< pair<int, string>* > c;
as member data object.

I need to use operator>> to store values into this container object
and
operator<< to print the contents of the container.

I have written both these operators as non-friend functions.

The destructor deletes all the pair<int, string> pointers in the
vector.

My doubt is that I am using a store_pair() function which allocates
memory
for each pair<int, string> pointer and stores into the container; but
the
dtor deletes these pointers thereby releasing the memory.

Kindly let me know if the approach in this program is correct.

It is one option, but consider just storing small objects (like your
pair<int, string>) in the vector rather than pointers. Then you won't
have to bother deleting them or copying a temporary object in your inserter.

 

[osama178]

Did you have to quote the entire message just to add that?

opps. I could swear I did not quote the message at all, but I have to
learn to double-check for next time.

Thanks for the tip.

 

[Ian Collins]

opps. I could swear I did not quote the message at all, but I have to
learn to double-check for next time.

Thanks for the tip.

OK, while we're here, it's bad form to quote signatures (the bit after
the "-- ").

 

[osama178]

OK, while we're here, it's bad form to quote signatures (the bit after

the "-- ").

I hope I got it right this time. Ok, I'll read the FAQ guidelines
again:
http://www.parashift.com/c++-faq-lite/how-to-post.html

 

[James Kanze]

Consider a class that has
vector< pair<int, string>* > c;
as member data object.

I need to use operator>> to store values into this container
object and operator<< to print the contents of the container.

I have written both these operators as non-friend functions.

The destructor deletes all the pair<int, string> pointers in
the vector.

My doubt is that I am using a store_pair() function which
allocates memory for each pair<int, string> pointer and stores
into the container; but the dtor deletes these pointers
thereby releasing the memory.

And what do you doubt about that? (I have doubts about the
wisdom of storing pointers, rather than the objects themselves,
given that std:air has value semantics, but that's a different
issue.)

Kindly let me know if the approach in this program is correct.

Here is the full program:

#include <cstdlib>
#include <iostream>
#include <vector>
#include <utility>

using namespace std;

class Test
{
public:
Test();
~Test();

typedef pair<int, string> pair_type;
typedef vector<pair_type*> container_type;

void store_pair(const pair_type& arg);
const container_type& container() const { return c; }

I also have a lot of doubts about exposing the implementation
like this. If you're going to do this, you might as well make
the container public, and be done with it.

private:
container_type c;
};

Test::Test() : c()
{
}

void Test::store_pair(const pair_type& arg)
{
pair_type* p = new pair_type(arg.first, arg.second);
c.push_back(p);

What's wrong with simply:
c.push_ back( new pair_type( arg ) ) ;
?

}

Test::~Test()
{
for (container_type::iterator it = c.begin(); it != c.end(); +
+it)
{
delete *it;
*it = 0;

Formally (but only formally), this is undefined behavior. You
must set the pointer in the container to null before the delete.

Practically, of course, there'll never be an implementation
where it will cause any problems, so I wouldn't worry about it.
For that matter, I wouldn't worry about setting the pointer to
null, since there'll never be an implementation where not doing
so will cause problems either, so I wouldn't worry about that
either.

}
}

istream& operator>>(istream& is, Test& obj)
{
while (is)
{
int x;
string str;
is >> x >> str;

if (is)
{
Test:air_type temp(x, str);
obj.store_pair(temp);
}
}
return is;
}

The real question is what semantics you want. Generally
speaking, I have my doubts about the format you're inputting; I
can't think of a context where it would be appropriate. But
what is appropriate depends on the application. Until you've
defined the format, it's impossible to say whether this is
correct or not.

ostream& operator<<(ostream& os, const Test& obj)
{
for (Test::container_type::const_iterator it =
obj.container().begin();
it != obj.container().end();
++it)
os << (**it).first << " " << (*it)->second << endl;

Why the two different ways to access the pair? Choose one, and
use it consistently. (I'd use the second, but that's more a
question of personal preference.)

 

[Triple-DES]

Formally (but only formally), this is undefined behavior.  You
must set the pointer in the container to null before the delete.

Why is that?

 

[James Kanze]

Why is that?

Because all objects in a container must be copiable, and a
deleted pointer is not copiable. In practice, of course

-- I don't know of any implementation today where copying a
deleted pointer will actually cause problems (although some
have definitely existed in the past), and above all

-- the implementation of std::vector is not going to copy an
element without reason, and as long as you don't extend the
vector, or explicitly do something which reads the value,
there's no reason.

So while I wouldn't leave a deleted pointer in an std::vector
which will remain live, I don't think you have to worry about
any problems between deleting it and assigning null, if they
immediately follow one another, nor any problems if the delete's
are preliminary to calling the vector's destructor (as is the
case here---there's no real reason to even assign 0 in this
case).

 

[subramanian100in]

(e-mail address removed), India wrote:

I also have a lot of doubts about exposing the implementation
like this. If you're going to do this, you might as well make
the container public, and be done with it.

By saying 'Exposing implementation', are you referring to
"const container_type& container() const { return c; }" ?

If so, I am returning only 'const container_type&'.
Will that create any problem ?

An alternative that I can think of is to make operator<<
a friend because, only in operator<<, container() is needed.
But Stroustrup has advised to avoid friend functions(except
to avoid global data and public data).

Please explain.

Since I am a learner, your suggestions are valuable to me.


Thanks
V.Subramanian

 

[Triple-DES]

Because all objects in a container must be copiable, and a
deleted pointer is not copiable.  In practice, of course

[snip]

You are right. 3.7.3.2/4 does indeed say that "The effect of using an
invalid pointer value (...) is undefined". But doesn't this also mean
that formally, setting the pointer to null is also undefined, because
"An object (...) is _used_ if its name appears in a potentially-
evaluated expression" (3.2/2)?

int * p = new int(5);
delete p;
p = 0; // undefined
p = new int(42); // reusing the ptr, fairly common, but undefined
char buf[sizeof(p)]; // ok, p is not _used_

Or am I missing something?

 

[James Kanze]

Because all objects in a container must be copiable, and a
deleted pointer is not copiable. In practice, of course

[snip]

You are right. 3.7.3.2/4 does indeed say that "The effect of using an
invalid pointer value (...) is undefined". But doesn't this also mean
that formally, setting the pointer to null is also undefined, because
"An object (...) is _used_ if its name appears in a potentially-
evaluated expression" (3.2/2)?

int * p = new int(5);
delete p;
p = 0; // undefined
p = new int(42); // reusing the ptr, fairly common, but undefined
char buf[sizeof(p)]; // ok, p is not _used_

Or am I missing something?

The fact that using an object doesn't necessarily mean using its
value? In general, I'd say that you only use its value if there
is either an lvalue to rvalue conversion, or an operation that
is described in terms of other operations which would involve an
lvalue to rvalue conversion. (That latter sounds a bit
complicated, but all it's saying is that ++p would use the
value, because it is defined as the equivalent of p = p + 1, and
the second use of p there involves an lvalue to rvalue
conversion.)

Historically, this rule dates back from the time when machines
had separate address registers, and loading an invalid address
into an address register could trap. Except that it's not
really that historical, since at least one machine still being
sold today today supports 48 bit pointers, and if the upper 16
bits aren't valid, loading the address could trigger a hardware
exception of some sort. So if the implementation of delete
returns the memory to the OS, and the OS unmaps it, just copying
the pointer, or reading it in any other way, could trap.

 

[James Kanze]

By saying 'Exposing implementation', are you referring to
"const container_type& container() const { return c; }" ?

If so, I am returning only 'const container_type&'.
Will that create any problem ?

It locks you into the actual container_type. Which should be
(probably) an implementation detail. (Of course, you've
typedef'ed it, so the user code should only count on the
guarantees you've specified for container_type.)

An alternative that I can think of is to make operator<<
a friend because, only in operator<<, container() is needed.

Exactly. And you're the author of operator<<. So there's
absolutely no need to let the client code in on the secret (and
risk having it depend on it).

But Stroustrup has advised to avoid friend functions(except
to avoid global data and public data).

Well, that's just a sort of general advice. You certainly
shouldn't make just any and every function a friend. But in
this case, I'd more or less consider the operator<< as a member
(since you write it as part of the class implementation), but
called with a different syntax. Using friend in such cases
generally increases encapsulation and coherence, which is a good
thing. Anyhow, if the choice is exposing some otherwise
unnecessary detail to client code, or making a function which is
really part of my public interface a friend, I'll go with the
latter every time.

Another possibility---one I actually use a lot---is to provide a
member function print(std:stream&), and have operator<< call
it. To tell the truth, however, I suspect that in my case, it's
more a case of ingrained habit, than for any good reason. One
of the first places where I used C++ had a coding guideline to
the effect that overloaded operators should always forward to a
named member function (probably to unnecessarily avoid the
friend), and since then, that's the way I do it. (It's a useful
idiom when inheritance is involved. operator<< can't be
virtual, since it can't be a member. Test:rint can be.)

 

[Triple-DES]

You are right. 3.7.3.2/4 does indeed say that "The effect of using an
invalid pointer value (...) is undefined". But doesn't this also mean
that formally, setting the pointer to null is also undefined, because
"An object (...) is _used_ if its name appears in a potentially-
evaluated expression" (3.2/2)?
int * p = new int(5);
delete p;
p = 0; // undefined
p = new int(42); // reusing the ptr, fairly common, but undefined
char buf[sizeof(p)]; // ok, p is not _used_
Or am I missing something?

The fact that using an object doesn't necessarily mean using its
value?  In general, I'd say that you only use its value if there
is either an lvalue to rvalue conversion, or an operation that
is described in terms of other operations which would involve an
lvalue to rvalue conversion.  (That latter sounds a bit
complicated, but all it's saying is that ++p would use the
value, because it is defined as the equivalent of p = p + 1, and
the second use of p there involves an lvalue to rvalue
conversion.)

This makes perfect sense. Pete Becker also pointed this out. But the
standard does not explicitly specify what it means to use the object's
value as opposed to using the object itself. I think 3.7.3.2/4 as it
is does leave some room for interpretation.

 

[subramanian100in]

Hello everyone,

kindly bear with me for asking same question. I am unclear now.

For deleting the elements of a container where the container contains
pointer to elements, I thought following is the only way.

for (container_type::iterator it = c.begin(); it != c.end(); ++it)
{
delete *it;
*it = 0;
}

Does 'delete *it; followed by *it = 0;' invoke undefined behaviour ?
If so, how else will I delete the pointers in the container ?

Also, all along I have been using

int* p = new int();
// use p
delete p;
p = 0;
Is this wrong then ?

Setting the pointer to null after deletion ensures that if the object
pointed to by the pointer is used in any way, then the program will
crash at the same point during every run so that debugging is easy. Am
I correct?

Kindly reply.

Thanks
V.Subramanian

 

[subramanian100in]

I take your good suggestion of keeping a print(ostream&) method inside
the class to avoid both exposing the implementation(that is returning
const contianer&) as well as avoid friend function.(If print() made
virtual, it can also help in polymorphism).

Now can I do the same to the input operator>> ?

ie

istream& operator>>(istream& is, Test& obj)
{
while (is)
{
int x;
string str;
is >> x >> str;

if (is)
{
Test:air_type temp(x, str);
obj.store_pair(temp);
}
}

return is;

}

That is, I should have a member function 'void Test::read(istream&)'
which should be called inside istream& operator>>(istream& is, const
Test& obj);

That is, operator>> will now become:

istream& operator>>(istream& is, const Test& obj)
{
obj.read(is);
return is;
}

Kindly reply.

Thanks
V.Subramanian

 

[Triple-DES]

Hello everyone,

kindly bear with me for asking same question. I am unclear now.

For deleting the elements of a container where the container contains
pointer to elements, I thought following is the only way.

for (container_type::iterator it = c.begin(); it != c.end(); ++it)
{
       delete *it;
       *it = 0;

}

Does 'delete *it; followed by *it = 0;' invoke undefined behaviour ?

Yes but only formally. See James Kanze's reply.

If so, how else will I delete the pointers in the container ?

Also, all along I have been using

int* p = new int();
// use p
delete p;
p = 0;
Is this wrong then ?

This is ok, but often unecessary. My example was a theoretical one,
based on one possible interpretation of the standard. I am sure that
the intent of the standard is to allow this.

Setting the pointer to null after deletion ensures that if the object
pointed to by the pointer is used in any way, then the program will
crash at the same point during every run so that debugging is easy. Am
I correct?

Only if it is accessed through the pointer that you set to null (if
you dereference the null pointer).

void f(int * p)
{
delete p;
p = 0; // redundant, p is not dereferenced before its lifetime ends
}

int main()
{
f(new int);
}