Style: use of member references in derived classes?

[Andy Lomax]

I'm deriving from a base class in a library, and the library vendor is
expecting me to add a 'string&' member in my derived class. Or, to be
more precise, they have provided two virtual functions:

1 void setstr(const std::string& str)
2 std::string& getstr(void)

So my obvious implementation is:

class foo : vendor_bar {
string& str;
public:
virtual void setstr(const string& str_) { str = str_; }
virtual string& getstr(void) { return str; }
};

I've got a bad feeling about this. Is it a good idea to have a
reference member like this? I can't think of any specific problems,
but I'd rather use a smart pointer of some sort.

Another factor is that the string is actually originally provided by
me (the library user), isn't used elsewhere in the library, and simply
comes back to me in the 'setstr' call; I use it myself further down in
the hierarchy. Given this, is there some better way that the library
could have been designed, rather than hard-wiring in a string
implementation?

Cheers

AL

[Alf P. Steinbach]

* Andy Lomax:

I'm deriving from a base class in a library, and the library vendor is
expecting me to add a 'string&' member in my derived class. Or, to be
more precise, they have provided two virtual functions:

1 void setstr(const std::string& str)
2 std::string& getstr(void)
Bad design.

So my obvious implementation is:

class foo : vendor_bar {
string& str;
public:
virtual void setstr(const string& str_) { str = str_; }
virtual string& getstr(void) { return str; }
};

Change that member declaration to just

string str;

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[Andy Lomax]

On Fri, 24 Jun 2005 10:07:50 GMT, al***@start.no (Alf P. Steinbach)
wrote:

* Andy Lomax:

I'm deriving from a base class in a library, and the library vendor is
expecting me to add a 'string&' member in my derived class. Or, to be
more precise, they have provided two virtual functions:

1 void setstr(const std::string& str)
2 std::string& getstr(void)

Bad design.

Do you have any specific reasons for saying that it's bad?

So my obvious implementation is:

class foo : vendor_bar {
string& str;
public:
virtual void setstr(const string& str_) { str = str_; }
virtual string& getstr(void) { return str; }
};

Change that member declaration to just

string str;

Hmm... problem is, this class may be instantiated thousands of times,
all containing exactly the same string. Any other ideas?

Thanks -

AL

[John Carson]

"Andy Lomax" <ab***@[127.0.0.1]> wrote in message
news:ea********************************@4ax.com

On Fri, 24 Jun 2005 10:07:50 GMT, al***@start.no (Alf P. Steinbach)
wrote:

* Andy Lomax:

I'm deriving from a base class in a library, and the library vendor
is expecting me to add a 'string&' member in my derived class. Or,
to be more precise, they have provided two virtual functions:

1 void setstr(const std::string& str)
2 std::string& getstr(void)

Bad design.

Do you have any specific reasons for saying that it's bad?

So my obvious implementation is:

class foo : vendor_bar {
string& str;
public:
virtual void setstr(const string& str_) { str = str_; }
virtual string& getstr(void) { return str; }
};

Change that member declaration to just

string str;

Hmm... problem is, this class may be instantiated thousands of times,
all containing exactly the same string. Any other ideas?

If you only want one string for the whole class, then the obvious thing to
do is make it a static member.

--
John Carson

[Andy Lomax]

On Fri, 24 Jun 2005 22:10:22 +1000, "John Carson"
<jc****************@netspace.net.au> wrote:

If you only want one string for the whole class, then the obvious thing to
do is make it a static member.

Sorry, not making myself clear. The class may be instantiated
thousands of times, but there may be only 4 or 5 unique values for the
string; I can't tell before runtime.

Ideally, I'd like to declare a smart pointer, but I can't see how to
tie that in with the prototypes that the library vendor has given me.
I can create a smart ptr for the string when I first create the
string, before passing it into the library as a reference. However,
when the library eventually calls 'setstr' with this reference, I
can't turn it back into the original smart ptr without some sort of
search.

Cheers

AL

[Shaun]

> I'm deriving from a base class in a library, and the library vendor is

expecting me to add a 'string&' member in my derived class. Or, to be
more precise, they have provided two virtual functions: 1 void setstr(const std::string& str)
2 std::string& getstr(void) Bad design.

These are NOT badly designed methods. They are, in fact, recommended.
Rather than rant on why, I'll point you to Herb Sutter's "Exceptional
C++" series.

I DO agree, however, that you just want to declare for your member:
string str;

The reason is that you'll be taking, in the case of the setter, a const
reference. The strings copy contructor will not be called and will
therefore cause undue overhead during the assignment operation.
Likewise for the getter; you'll return a reference (maybe should be a
const ref) and avoid copy contruction on the return from the method.

[Shaun]

That should have read, "not cause undue overhead." I apologize for the
confusion.

[Shezan Baig]

Andy Lomax wrote:

I'm deriving from a base class in a library, and the library vendor is
expecting me to add a 'string&' member in my derived class. Or, to be
more precise, they have provided two virtual functions:

1 void setstr(const std::string& str)
2 std::string& getstr(void)

So my obvious implementation is:

class foo : vendor_bar {
string& str;
public:
virtual void setstr(const string& str_) { str = str_; }
virtual string& getstr(void) { return str; }
};

This shouldnt even compile! What does 'str' refer to?

[Shaun]

You're absolutely correct -- if you assume that the constructor didn't
assign (and take) a std::string& for an argument. It will compile,
however, if the class is defined like so:

....

class SomeStrClass
{
private:
std::string& m_str;
public:
SomeStrClass(std::string& init_str)
: m_str(init_str)
{
}

~SomeStrClass(void)
{
}

void set_str(const std::string& str)
{
m_str = str;
}

const std::string& get_str(void) const
{
return m_str;
}
};
Note that changing the constructor to the following will NOT work:

SomeStrClass(void)
: m_str("")
{
}

The reason (if its not obvious) is that you cannot assign a reference
to a temporary. You'd end up with a "what does this string refer to"
type situation like your question asks.

[Andy Lomax]

On 24 Jun 2005 07:25:06 -0700, "Shaun" <se******@gvtc.com> wrote:

Bad design.

These are NOT badly designed methods. They are, in fact, recommended.
Rather than rant on why, I'll point you to Herb Sutter's "Exceptional
C++" series.

Already got 6 C++ and STL books, with a 7th in the post, so I'm afraid
I'll have to pass up on that. It'll take me to the next ice age just
to read this lot.

I don't suppose you could summarise? :)

Cheers

AL

[Shaun]

"I don't suppose you could summarise?"

Now that I think of it it may be Scott Meyers book, "Effective C++" (if
not both books), however the guidline is generally stated as follows:

"prefer pass-by-reference to pass-by-value"

you can pair this with another oft-stated guidline, "use const whenever
possible".

As to WHY these guidlines are states as they are, I don't have my own
copies nearby and so won't risk improperly parapharasing or putting
words in the authors' mouths but you may find some good discussion on
the matter elsewhere on the web. I provide tree links below to get you
started.

http://www.informit.com/articles/art...73338&seqNum=2
http://www.goingware.com/tips/parame...arameters.html
http://www.tantalon.com/pete/cppopt/asyougo.htm

[John Carson]

"Andy Lomax" <ab***@[127.0.0.1]> wrote in message
news:8b********************************@4ax.com

On Fri, 24 Jun 2005 22:10:22 +1000, "John Carson"
<jc****************@netspace.net.au> wrote:

If you only want one string for the whole class, then the obvious
thing to do is make it a static member.

Sorry, not making myself clear. The class may be instantiated
thousands of times, but there may be only 4 or 5 unique values for the
string; I can't tell before runtime.

Ideally, I'd like to declare a smart pointer, but I can't see how to
tie that in with the prototypes that the library vendor has given me.
I can create a smart ptr for the string when I first create the
string, before passing it into the library as a reference. However,
when the library eventually calls 'setstr' with this reference, I
can't turn it back into the original smart ptr without some sort of
search.

I don't know what the smartness of the pointer has to do with anything.

If you think it is worth the trouble (which seems doubtful to me), it seems
that what you need is an auxiliary structure (say, a set since you can
insert without invalidating addresses). You make the set a static member of
the class and each object of the class has a pointer to an entry in this
set. Try the following:

#include<string>
#include<set>
#include <iostream>
using namespace std;

class vendor_bar
{};

class foo : vendor_bar {
string* pstr;
static set<string> s;
public:
virtual void setstr(const string& str_)
{ pstr = &(*s.insert(str_).first); }
virtual string& getstr(void) { return *pstr; }
};

set<string> foo::s;

// Note that, rather helpfully, set::insert will only make
// an actual insertion if the string isn't already there.
// If it is there, it just returns an iterator to the existing element.

int main()
{
const int number=8;
foo f[number];
f[0].setstr("First");
f[1].setstr("First");
f[2].setstr("Second");
f[3].setstr("Second");
f[4].setstr("Third");
f[5].setstr("Fourth");
f[6].setstr("Fifth");
f[7].setstr("Fifth");

for(int i=0; i<number; ++i)
cout << "i: " << i << " string is: " << f[i].getstr() << endl;
}

You could further complicate this by refcounting the entries in the set and
removing an entry when no object stored its address, but this seems unlikely
to be worth it.

--
John Carson

[Andy Lomax]

Thanks - I'll check the references. I think your other reference was
probably Meyer #20 - 'Prefer pass-by-reference-to-const to
pass-by-value'; I found that this morning.

Cheers

AL

[Alf P. Steinbach]

* Shaun:

"I don't suppose you could summarise?"

Now that I think of it it may be Scott Meyers book, "Effective C++" (if
not both books), however the guidline is generally stated as follows:

"prefer pass-by-reference to pass-by-value"

you can pair this with another oft-stated guidline, "use const whenever
possible".

You remember the first one incorrectly (unless Scott Meyers has written in
his sleep); the second is advice I've given myself, publicly.

Anyway it doesn't have anything to do with the interface Andy was given:

void setstr(const std::string& str)
std::string& getstr(void)

It's badly designed, period.

It also indicates that the person writing it is more familiar with C than
C++.

The only way it could make sense would be for 'getstr' to return a reference
to a non-per-instance member, e.g. a static member or global.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?