Strings and refs, need comments on code

[pmatos]

Hi all,

I've been having questions about strings, references,
initializations... I've created code (which will not compile due to a
reference problem) and I'd like comments on why this won't work and any
other comment to the code with efficiency in mind.

#include <iostream>
#include <string>

using std::cout;
using std::string;

class myClass {
public:
myClass(string s) : str(s) { }
~myClass() { }

string& getStr() const { return str; }

private:
string str;
};

int main() {

string * s = new string("FOO");
myClass * m = new myClass(*s);

cout << m->getStr() << std::endl;

delete s;
delete m;

return 0;
}

Some questions:
1 - Should constructor receive a &? (would it be faster? now, it's
copying the string in str(s), right?)
2 - why returning string& is not working? (returning just string would
return a copy of str, right?)
3 - How can I return the string, as efficiently as possible but making
sure it cannot be modified?

Any other comments are welcome.

Thanks,

Paulo Matos

[Victor Bazarov]

pmatos wrote:

I've been having questions about strings, references,
initializations... I've created code (which will not compile due to a
reference problem) and I'd like comments on why this won't work and any
other comment to the code with efficiency in mind.

#include <iostream>
#include <string>

using std::cout;
using std::string;

class myClass {
public:
myClass(string s) : str(s) { }
Definitely should be

myClass(string const& s) : str(s) { }
~myClass() { }

string& getStr() const { return str; }
If your object is 'const', you can't return a non-const reference
to a member. It breaks the assumption that the object is const.

private:
string str;
};

int main() {

string * s = new string("FOO");
myClass * m = new myClass(*s);
You really shouldn't use dynamic memory unless you _have_to_. Just
plain

myClass m("FOO");

will work fine in this particular case.

cout << m->getStr() << std::endl;
If you use a local object, you'll have to change to m.getStr().

delete s;
delete m;
.... and there will be no need to delete anything.

return 0;
}

Some questions:
1 - Should constructor receive a &? (would it be faster? now, it's
copying the string in str(s), right?)
It should receive a reference, but it should still copy it locally.
As originally posted, two strings are created instead of one.
2 - why returning string& is not working? (returning just string would
return a copy of str, right?)
Right. Just return a 'string const&'.
3 - How can I return the string, as efficiently as possible but making
sure it cannot be modified?

Return by a reference to const.

V

[Phil Endecott]

pmatos wrote:

Hi all,

I've been having questions about strings, references,
initializations... I've created code (which will not compile due to a
reference problem) and I'd like comments on why this won't work and any
other comment to the code with efficiency in mind.
I believe that strings generally have efficient implementations. It
does depend on your library, but they should normally have copy-on-write
reference counting, or something of that sort. I'm sure someone will
correct me if I'm wrong, but I believe that it is reasonable to pass
strings around with as little care about efficiency as if you were
passing ints or chars.
#include <iostream>
#include <string>

using std::cout;
using std::string;

class myClass {
public:
myClass(string s) : str(s) { }
~myClass() { }

string& getStr() const { return str; }

private:
string str;
};

int main() {

string * s = new string("FOO");
myClass * m = new myClass(*s);

cout << m->getStr() << std::endl;

delete s;
delete m;

return 0;
}

Some questions:
1 - Should constructor receive a &? (would it be faster? now, it's
copying the string in str(s), right?)
See above for strings. But more generally, yes, if you are passing
something like a large struct or a container you can pass it as a const
T& (const is important) and get improved efficiency.
2 - why returning string& is not working? (returning just string would
return a copy of str, right?)
Give us a clue, what is the error message?
3 - How can I return the string, as efficiently as possible but making
sure it cannot be modified?

See above for strings. But more generally, yes, this is something you
might need to worry about for large types, and it is a bit more painful
than passing parameters because you have to worry more about the
lifetime of the objects. In my code I often use "out parameters", i.e.
I require that the caller declares an (empty) variable for the result
and pass it as a reference parameter. Other options include smart
pointers. I believe that you will find some useful material in the FAQ.

--Phil.

[Andre Kostur]

Victor Bazarov <v.********@comAcast.net> wrote in news:BJCpe.81519
$N********@newsread1.mlpsca01.us.to.verio.net:

pmatos wrote:

I've been having questions about strings, references,
initializations... I've created code (which will not compile due to a
reference problem) and I'd like comments on why this won't work and any
other comment to the code with efficiency in mind.

#include <iostream>
#include <string>

using std::cout;
using std::string;

class myClass {
public:
myClass(string s) : str(s) { }

Definitely should be

myClass(string const& s) : str(s) { }

~myClass() { }

string& getStr() const { return str; }

If your object is 'const', you can't return a non-const reference
to a member. It breaks the assumption that the object is const.

private:
string str;
};

int main() {

string * s = new string("FOO");
myClass * m = new myClass(*s);

You really shouldn't use dynamic memory unless you _have_to_. Just
plain

myClass m("FOO");

will work fine in this particular case.

cout << m->getStr() << std::endl;

If you use a local object, you'll have to change to m.getStr().

delete s;
delete m;

... and there will be no need to delete anything.

You forgot to mention... and no need to delete anything in a particular
order. (OK, 'need' may be a little strong, but we'll stick with it for
now). As it is now, *m is holding a reference to *s. Between "delete s;"
and "delete m;", m is a timebomb waiting to go off. It now has a reference
to an invalid object. In a more complex program where these two delete
statements aren't next to each other, one may be tempted to try using the
*m object to access the string it references......

return 0;
}

Some questions:
1 - Should constructor receive a &? (would it be faster? now, it's
copying the string in str(s), right?)

It should receive a reference, but it should still copy it locally.
As originally posted, two strings are created instead of one.

2 - why returning string& is not working? (returning just string would
return a copy of str, right?)

Right. Just return a 'string const&'.

3 - How can I return the string, as efficiently as possible but making
sure it cannot be modified?

Return by a reference to const.

V

[Victor Bazarov]

Andre Kostur wrote:

Victor Bazarov <v.********@comAcast.net> wrote in news:BJCpe.81519
$N********@newsread1.mlpsca01.us.to.verio.net:

pmatos wrote:

I've been having questions about strings, references,
initializations... I've created code (which will not compile due to a
reference problem) and I'd like comments on why this won't work and any
other comment to the code with efficiency in mind.

#include <iostream>
#include <string>

using std::cout;
using std::string;

class myClass {
public:
myClass(string s) : str(s) { }
Definitely should be

myClass(string const& s) : str(s) { }

~myClass() { }

string& getStr() const { return str; }

If your object is 'const', you can't return a non-const reference
to a member. It breaks the assumption that the object is const.

private:
string str;
};

int main() {

string * s = new string("FOO");
myClass * m = new myClass(*s);

You really shouldn't use dynamic memory unless you _have_to_. Just
plain

myClass m("FOO");

will work fine in this particular case.

cout << m->getStr() << std::endl;

If you use a local object, you'll have to change to m.getStr().

delete s;
delete m;

... and there will be no need to delete anything.

You forgot to mention... and no need to delete anything in a particular
order. (OK, 'need' may be a little strong, but we'll stick with it for
now). As it is now, *m is holding a reference to *s.

Huh? '*m' is an object of class myClass, which happens to have 'str' as
its only data member, and it's not a reference...
Between "delete s;"
and "delete m;", m is a timebomb waiting to go off. It now has a reference
to an invalid object. In a more complex program where these two delete
statements aren't next to each other, one may be tempted to try using the
*m object to access the string it references......
You need to look at the original post again, I believe.

return 0;
}

V

[Kristo]

Phil Endecott wrote:

pmatos wrote:

Hi all,

I've been having questions about strings, references,
initializations... I've created code (which will not compile due to a
reference problem) and I'd like comments on why this won't work and any
other comment to the code with efficiency in mind.

I believe that strings generally have efficient implementations. It
does depend on your library, but they should normally have copy-on-write
reference counting, or something of that sort. I'm sure someone will
correct me if I'm wrong, but I believe that it is reasonable to pass
strings around with as little care about efficiency as if you were
passing ints or chars.

That may be true, but why take the chance that it isn't? See below.

#include <iostream>
#include <string>

using std::cout;
using std::string;

class myClass {
public:
myClass(string s) : str(s) { }
~myClass() { }

string& getStr() const { return str; }

private:
string str;
};

int main() {

string * s = new string("FOO");
myClass * m = new myClass(*s);

cout << m->getStr() << std::endl;

delete s;
delete m;

return 0;
}

Some questions:
1 - Should constructor receive a &? (would it be faster? now, it's
copying the string in str(s), right?)

See above for strings. But more generally, yes, if you are passing
something like a large struct or a container you can pass it as a const
T& (const is important) and get improved efficiency.

I don't think you should make an exception for std::string. The whole
point of abstraction is so you don't have to know whether or not the
class' implementation is "fast enough." Be consistent, and pass
references (to const if possible) whenever you're dealing with
*anything* other than a primitive type.

2 - why returning string& is not working? (returning just string would
return a copy of str, right?)

Give us a clue, what is the error message?

He has a const function returning a non-const reference to internal
data. That isn't allowed.

3 - How can I return the string, as efficiently as possible but making
sure it cannot be modified?

See above for strings. But more generally, yes, this is something you
might need to worry about for large types, and it is a bit more painful
than passing parameters because you have to worry more about the
lifetime of the objects. In my code I often use "out parameters", i.e.
I require that the caller declares an (empty) variable for the result
and pass it as a reference parameter. Other options include smart
pointers. I believe that you will find some useful material in the FAQ.

I think Victor's response to this was much simpler: return a reference
to const.

Kristo

[Phlip]

pmatos wrote:

string * s = new string("FOO");
myClass * m = new myClass(*s);
Don't use 'new' without a major reason. "I need an object inside this
function" is no reason. Just construct the object directly:

string s = "FOO";
1 - Should constructor receive a &? (would it be faster? now, it's
copying the string in str(s), right?)

When starting a project, never worry about program speed. Do worry about
programmer speed. Programmers should always use the similar constructions at
similar places, just to preserve legibility. If everything else used 'const
&' and this one did not, that should raise an unanswerable question, why is
this different?

Use 'const &' because big objects need that to be fast.

--
Phlip
http://www.c2.com/cgi/wiki?ZeekLand

[Andre Kostur]

Victor Bazarov <v.********@comAcast.net> wrote in
news:px*******************@newsread1.mlpsca01.us.t o.verio.net:

Andre Kostur wrote:

Victor Bazarov <v.********@comAcast.net> wrote in news:BJCpe.81519
$N********@newsread1.mlpsca01.us.to.verio.net:

pmatos wrote:

I've been having questions about strings, references,
initializations... I've created code (which will not compile due to
a reference problem) and I'd like comments on why this won't work
and any other comment to the code with efficiency in mind.

#include <iostream>
#include <string>

using std::cout;
using std::string;

class myClass {
public:
myClass(string s) : str(s) { }

Definitely should be

myClass(string const& s) : str(s) { }
~myClass() { }

string& getStr() const { return str; }

If your object is 'const', you can't return a non-const reference
to a member. It breaks the assumption that the object is const.
private:
string str;
};

int main() {

string * s = new string("FOO");
myClass * m = new myClass(*s);

You really shouldn't use dynamic memory unless you _have_to_. Just
plain

myClass m("FOO");

will work fine in this particular case.
cout << m->getStr() << std::endl;

If you use a local object, you'll have to change to m.getStr().
delete s;
delete m;

... and there will be no need to delete anything.

You forgot to mention... and no need to delete anything in a
particular order. (OK, 'need' may be a little strong, but we'll
stick with it for now). As it is now, *m is holding a reference to
*s.

Huh? '*m' is an object of class myClass, which happens to have 'str'
as its only data member, and it's not a reference...

Ooops.... my bad. Somehow I figured the private member was a string&.
Completely my error. Must clean my eyeballs.

I agree with your original points.... why bother to dynamically allocate
when you're (the OP) treating it as a local variable anyway.