Question on Myers #30

I

Ioannis Vranos

Alf said:
void client( bool thisOrThat )
{
std::string const gwb = "Bah!";
std::string const* ps = thisOrThat? &gwb : &iv.getstring();

std::cout << *ps << std::endl;
}


Making it more complete:

#include <iostream>

class SomeClass
{
std::string s;

public:
SomeClass():s("Test string") {}

const std::string &getstring() const { return s; }
};

void client( bool thisOrThat )
{
SomeClass iv;

std::string const gwb = "Bah!";
std::string const* ps = thisOrThat? &gwb : &iv.getstring();

std::cout << *ps << std::endl;
}


int main()
{
client(true);
}


Yes, if you convert const std::string & to const std::string you end up
getting the address of a temporary and all my compilers warn of this.


However this is an unusual use at first, to use a pointer to a returned
const string &, and the user as it seems can still take the address of
the temporary even if you return a temporary since the beginning.


If you want to make more obvious that this should not happen, you can
add to the documentation that the const references or const pointers
returned by these methods point to an implementation specific space that
can change after that call.


This is the case of all facilities of <ctime> BTW. All pointers returned
point to implementation-specific areas and can change after the call.
 
I

Ioannis Vranos

Ioannis said:
Making it more complete:

#include <iostream>

class SomeClass
{
std::string s;

public:
SomeClass():s("Test string") {}

const std::string &getstring() const { return s; }
};

void client( bool thisOrThat )
{
SomeClass iv;

std::string const gwb = "Bah!";
std::string const* ps = thisOrThat? &gwb : &iv.getstring();

std::cout << *ps << std::endl;
}


int main()
{
client(true);
}


Yes, if you convert const std::string & to const std::string you end up
getting the address of a temporary and all my compilers warn of this.


However this is an unusual use at first, to use a pointer to a returned
const string &, and the user as it seems can still take the address of
the temporary even if you return a temporary since the beginning.


If you want to make more obvious that this should not happen, you can
add to the documentation that the const references or const pointers
returned by these methods point to an implementation specific space that
can change after that call.


This is the case of all facilities of <ctime> BTW. All pointers returned
point to implementation-specific areas and can change after the call.


A reference:

http://www.research.att.com/~bs/3rd_loc.pdf


Check on page 906:

"Beware: both localtime() and gmtime() return a tm * to a statically
allocated object; a subsequent call of that function will change the
value of that object. Either use such a return value immediately,
or copy the tm into storage that you control.

Similarly, asctime() returns a pointer to a statically allocated
character array."
 
H

Howard

Master of C++ said:
Hi,

In Effective C++, #30, Myers says:

"Avoid member functions that return pointers or references to members
less accessible than themselves"

But, operator[] does exactly the opposite. Is overloading operator[]
considered "evil" in C++ circles ? I write a lot of numerical
programming code, and such operators make life easier for me.

Thanks,
Vijay.

Perhaps you have an earlier edition? The 2nd edition of that book states
very clearly

"Item 30: Avoid member functions that return non-const pointers or
references to members less accesiable than themselves."

Notice the "non-const" part of that heading. That's the key to his whole
argument. He's saying that, if you return a non-const reference, then
you're in effect granting permission for the caller to modify member data
that you've already said they have no business touching. It's ok to return
a const reference, since then the data is not modifiable by the caller.
Similar arguments are made regarding pointers, with added details about
calling member functions with less visibility.

-Howard
 
J

Jonathan Turkanis

Howard said:
Notice the "non-const" part of that heading. That's the key to his
whole argument. He's saying that, if you return a non-const
reference, then you're in effect granting permission for the caller
to modify member data that you've already said they have no business
touching.

The problem with this argument is that declaring a data member private by itself
does not mean that the client has no business "touching it." By providing a
function which returns a non-const reference to (part of) such a data member,
you can give the client an approved way to "touch" it. Forcing the client to use
an accessor function can still provide a degree of encapsulation; e.g. only part
of a data member can be exposed and the data can be relocated or renamed without
affecting the public interface.

Jonathan
 
H

Howard

Jonathan Turkanis said:
The problem with this argument is that declaring a data member private by
itself
does not mean that the client has no business "touching it." By providing
a
function which returns a non-const reference to (part of) such a data
member,
you can give the client an approved way to "touch" it. Forcing the client
to use
an accessor function can still provide a degree of encapsulation; e.g.
only part
of a data member can be exposed and the data can be relocated or renamed
without
affecting the public interface.

Jonathan

I'm not sure I understand if you're agreeing or disagreeing with what I've
said (or with what Mr. Meyers has written). Have you read the text I'm
referring to? The use of an accessor function is exactly what he shows. He
just says that it should not return a non-const reference or pointer to the
data. Obviously there are many things to consider when designing a class,
but the point of this *particular* reference is to not make such an accessor
function return a *non-const* reference, because returning a non-const
reference essentially allows the user direct access to the underlying data,
as if you had made the variable public in the first place.

-Howard
 
A

Alf P. Steinbach

* Howard:
I'm not sure I understand if you're agreeing or disagreeing with what I've
said (or with what Mr. Meyers has written). Have you read the text I'm
referring to? The use of an accessor function is exactly what he shows. He
just says that it should not return a non-const reference or pointer to the
data. Obviously there are many things to consider when designing a class,
but the point of this *particular* reference is to not make such an accessor
function return a *non-const* reference, because returning a non-const
reference essentially allows the user direct access to the underlying data,
as if you had made the variable public in the first place.

The 'at' member function of the standard containers is one counter-example.

As I wrote earlier, "Effective C++" are rules of thumbs.

This one is not a particularly good one.
 
H

Howard

Alf P. Steinbach said:
* Howard:

The 'at' member function of the standard containers is one
counter-example.

As I wrote earlier, "Effective C++" are rules of thumbs.

I agree, absolutely!
This one is not a particularly good one.

Again, I fail to see what the disagreement is (or what's not particularly
good about his advice here, to put it another way).

Here is a response I see from you in this thread:

<paste>
Ioannis: If you were returning a non-const reference(!) to your internal
data member, then you shouldn't do it.

Alf: Right.
</paste>

What I see Mr. Meyers saying is *exactly* what I see you agreeing to above:
that you shouldn't return a non-const reference to an internal data member.
Did I miss something?

-Howard
 
J

Jonathan Turkanis

Howard said:
I'm not sure I understand if you're agreeing or disagreeing with what
I've said

disagreeing

(or with what Mr. Meyers has written).

disagreeing
Have you read the
text I'm referring to?
Yes.

The use of an accessor function is exactly
what he shows. He just says that it should not return a non-const
reference or pointer to the data. Obviously there are many things to
consider when designing a class, but the point of this *particular*
reference is to not make such an accessor function return a
*non-const* reference, because returning a non-const reference
essentially allows the user direct access to the underlying data, as
if you had made the variable public in the first place.

In my original message I was trying to show that this argument is fallacious.

Providing a public accessor function returning a non-const reference is not the
same as making the member data public. Eg.:

class Thing {
public:
...
int& size() { return size_; }
const int& size() const { return size_; }
int& weight() { return weight_; }
const int& weight() const { return weight_; }
private:
...
int size_;
int weight_;
};

can be reimplemented

class Thing {
public:
....
int& size() { return size_weight_.first; }
const int& size() const { return size_weight_.first; }
int& weight() { return size_weight_.second; }
const int& weight() const { return size_weight_.second; }
private:
...
std::pair<int, int> size_weight_;
};

without changing the public interface of Thing. This would not be possible if
size had been exposed as a public data member of type int. This is one of the
traditional advantages of using accessor functions, and it is not diminished by
the fact that two of them return non-const references.

Jonathan
 
H

Howard

Jonathan Turkanis said:
disagreeing

(or with what Mr. Meyers has written).

disagreeing


In my original message I was trying to show that this argument is
fallacious.

Providing a public accessor function returning a non-const reference is
not the
same as making the member data public. Eg.:

class Thing {
public:
...
int& size() { return size_; }
const int& size() const { return size_; }
int& weight() { return weight_; }
const int& weight() const { return weight_; }
private:
...
int size_;
int weight_;
};

can be reimplemented

class Thing {
public:
....
int& size() { return size_weight_.first; }
const int& size() const { return size_weight_.first; }
int& weight() { return size_weight_.second; }
const int& weight() const { return size_weight_.second; }
private:
...
std::pair<int, int> size_weight_;
};

without changing the public interface of Thing. This would not be possible
if
size had been exposed as a public data member of type int. This is one of
the
traditional advantages of using accessor functions, and it is not
diminished by
the fact that two of them return non-const references.


Jonathan

But that's not his point! What he discusses is this:

class Y
{
public:
int y;
};

class X
{
Y yx; // This should be private!
public:
Y& gety() { return yx; }
};

int main()
{
X xmain;
Y& ymain = xmain.gety();
ymain.y = 3; // But now you're changing the contents of the *private*
member yx!
}

The non-const reference returned from get() allows the main function to
modify the contents of the private member of xmain.

Mr. Meyers is saying that, in effect, yx is no longer really private,
because it can be accessed by getting a non-const reference to it.
Returning a const reference instead, that would not happen.

-Howard
 
E

E. Mark Ping

The 'at' member function of the standard containers is one counter-example.

As I wrote earlier, "Effective C++" are rules of thumbs.

This one is not a particularly good one.

This is a particulary foolish example. Containers present an
interface to the contained data. vector::at and vector::eek:perator[]
dont' return non-const references to their *implementation* but to the
contained data, which is the purpose of the continer.

If (say) std::vector::capacity() returned a reference to the allocated
capacity, *that* would be a problem.
 
M

Master of C++

But on the other hand, a non-const operator[] is very useful in
situations where we have to write code such as:

a = b*c[j];
++v;

which, while breaking encapsulation momentarily, is very elegant and
succinct especially when using C++ for numerical programming. IMHO, as
long as the user of the class knows what he is doing, I am guessing
that it is OK to return non-const references to private data members.

-Vijay.
 
I

Ioannis Vranos

Master said:
But on the other hand, a non-const operator[] is very useful in
situations where we have to write code such as:

a = b*c[j];
++v;

which, while breaking encapsulation momentarily, is very elegant and
succinct especially when using C++ for numerical programming. IMHO, as
long as the user of the class knows what he is doing, I am guessing
that it is OK to return non-const references to private data members.



Ping above in the thread is right. The returned non-const reference is
not a reference to the internal implementation, but a reference to a
contained object in the container. It is part of a container's job to
provide this.
 
J

Jonathan Turkanis

Howard said:
"Jonathan Turkanis" <[email protected]> wrote in message
But that's not his point!

Were talking about "Avoid member functions that return pointers or references to
members
less accessible than themselves," right?
What he discusses is this:

class Y
{
public:
int y;
};

class X
{
Y yx; // This should be private!

And it is!
public:
Y& gety() { return yx; }
};

This is the same as my example, and I'm arguing that contrary to Meyers,
sometimes it's okay. I've given the argument twice now.

Jonathan
 
J

Jonathan Turkanis

E. Mark Ping said:
The 'at' member function of the standard containers is one
counter-example.

As I wrote earlier, "Effective C++" are rules of thumbs.

This one is not a particularly good one.

This is a particulary foolish example. Containers present an
interface to the contained data. vector::at and vector::eek:perator[]
dont' return non-const references to their *implementation* but to the
contained data, which is the purpose of the continer.

The contained data happens to be part of the implementation here, and a part
which it is okay to expose directly via operator[].

Jonathan
 
I

Ioannis Vranos

Jonathan said:
Were talking about "Avoid member functions that return pointers or references to
members
less accessible than themselves," right?


Actually this was corrected to:

"Item 30: Avoid member functions that return non-const pointers or
references to members less accessible than themselves."

by Howard above in the thread (a thing I was suspecting it was like it
since the beginning anyway).
 
J

Jonathan Turkanis

Ioannis said:
Actually this was corrected to:

"Item 30: Avoid member functions that return non-const pointers or
references to members less accessible than themselves."

by Howard above in the thread (a thing I was suspecting it was like it
since the beginning anyway).

I just copied and pasted from the OP. But my examples apply to the clarified
version just the same.

Jonathan
 
G

Gary Labowitz

Jonathan Turkanis said:
I just copied and pasted from the OP. But my examples apply to the clarified
version just the same.

In that case, I have to jump in and disagree.
There could be many reasons why a member is made private. Encapsulation and
data hiding work together in this case to allow the class to store and
manipulate the member as it sees fit. Consider member data that MUST be in a
range of 20-100 to make any sense. I would certainly hide this member,
making it private, and provide a public interface to assign to it. My
interface function would have checking in it to be sure that my constraints
on the range were met. If not, throw an exception. If, however, I also
wanted to allow the user to see the value of the member data I might well
return a constant reference from some accessor.

But to pass a non-const reference not only breaks encapsulation, it
endangers the object by losing control of the member range constraint. This
is the issue for me.
 
J

Jonathan Turkanis

Gary said:
In that case, I have to jump in and disagree.
There could be many reasons why a member is made private.
Encapsulation and data hiding work together in this case to allow the
class to store and manipulate the member as it sees fit. Consider
member data that MUST be in a range of 20-100 to make any sense. I
would certainly hide this member, making it private, and provide a
public interface to assign to it. My interface function would have
checking in it to be sure that my constraints on the range were met.
If not, throw an exception. If, however, I also wanted to allow the
user to see the value of the member data I might well return a
constant reference from some accessor.
But to pass a non-const reference not only breaks encapsulation, it
endangers the object by losing control of the member range
constraint. This is the issue for me.

I'm not arguing that all data should be public or virtually public! Just that it
*sometimes* makes sense to have a public accessor function which returns a
reference to a private member.

There are two separate issues:

1. Does it sometimes make sense to have public data? The answer is yes, with the
emphasis on 'sometimes.' Think of std::pair.

2. Is it sometimes okay to have a private member exposes as a non-const
reference by an accessor function? (I'm simplifying a bit by specifying the
access levels exactly).

If you think the answer to 1) is yes, then you can invent cases of 2) You simply
argue that by adding the indirection of an accessor function you're allowing
freedom to change the implementation later in ways which don't affect the public
interface.

There are probably more important counterexamples than this, but you only need
one counterexample.

Jonathan
 
A

Alf P. Steinbach

* Howard:
* Alf P. Steinbach:

Again, I fail to see what the disagreement is (or what's not particularly
good about his advice here, to put it another way).

The usual case is exactly opposite of Meyers' advice: usually your data
member's got too lenient accessibility if it's of the same accessibility as a
function that returns a reference to this.

Here is a response I see from you in this thread:

<paste>
Ioannis: If you were returning a non-const reference(!) to your internal
data member, then you shouldn't do it.

Alf: Right.
</paste>

That "it" was about changing reference to non-reference.

What I see Mr. Meyers saying is *exactly* what I see you agreeing to above:
that you shouldn't return a non-const reference to an internal data member.
Did I miss something?

Yes.
 
I

Ioannis Vranos

Jonathan said:
I'm not arguing that all data should be public or virtually public! Just that it
*sometimes* makes sense to have a public accessor function which returns a
reference to a private member.

There are two separate issues:

1. Does it sometimes make sense to have public data? The answer is yes, with the
emphasis on 'sometimes.' Think of std::pair.


std::pair, for example the one returned from map, contains copies of the
elements.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,766
Messages
2,569,569
Members
45,042
Latest member
icassiem

Latest Threads

Top