accessors

i have a question about class design, specifically regarding accessor
functions. which is probably a better design?

class foo
{
public:
void name(string const& n) { name_ = n; }
string name() const { return name_; }
private:
string name_;
};

class foo
{
public:
string& name() { return name_; }
string name() const { return name_; }
private:
string name_;
};

the difference from the client point of view is how you set the name:

foo f;
f.name("name");
// or
f.name() = "name";

the way i figure it, the first would make assignments explicit, and
prevent accidental assignments. however, when you consider *reading* the
name, you get a different story. there's no difference for simple types
(with fast copying/copy construction), but for expensive types you pay
for a copy construction (or at least something to that effect) using the
first option, even if you don't need it. with the second you only pay
for it on const objects (and i'll get to that next).

now, for integral types, it's not a cost issue, so there it's just a
style issue. but for code passing bigger things around, there is no
point paying for unnecessary copying if it can be avoided. given that
constraint, this would seem like the best option:

class foo
{
public:
string& name() { return name_; }
string const& name() const { return name_; }
private:
string name_;
};

are there any thoughts on this?

mark

* Mark A. Gibbs:
>
> i have a question about class design, specifically regarding accessor
> functions. which is probably a better design?
>
> class foo
> {
> public:
>     void name(string const& n) { name_ = n; }
>     string name() const { return name_; }
> private:
>     string name_;
> };
>
> class foo
> {
> public:
>     string& name() { return name_; }
>     string name() const { return name_; }
> private:
>     string name_;
> };
>
> the difference from the client point of view is how you set the name:
>
> foo f;
> f.name("name");
> // or
> f.name() = "name";

Both ugly.


> the way i figure it, the first would make assignments explicit

It doesn't.


> and prevent accidental assignments.

It doesn't.


> however, when you consider *reading* the
> name, you get a different story. there's no difference for simple types
> (with fast copying/copy construction), but for expensive types you pay
> for a copy construction (or at least something to that effect) using the
> first option, even if you don't need it. with the second you only pay
> for it on const objects (and i'll get to that next).

Before optimizing for speed and memory, measure.

Before measuring, check whether there really is a problem.

Before checking for that, use sound judgement in coding, such as
returning a member data item by reference to const.


> are there any thoughts on this?

'set_what' or 'setWhat' are good names for setting a 'what': use the
imperative form when naming an action to be performed, use a
description when naming a data item.

Return of reference to const object is a good but limited way to avoid
excessive copying.

A function that returns a reference to a member is equivalent to
exposing that member directly and is in general Ungood (TM).

--
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?

"Mark A. Gibbs" <x_gibbsmark@rogers.com_x> wrote in message news:rFyUc.1772127$Ar.1246605@t
wister01.bloor.is.net.cable.rogers.com...
>
> i have a question about class design, specifically regarding accessor
> functions. which is probably a better design?
>

Actually, I'd do:

const string& name() const { return name; }

This avoids the copy, and keeps people who grab the reference
without copying it to not change the underlying object.

"Mark A. Gibbs" <x_gibbsmark@rogers.com_x> wrote:

> i have a question about class design, specifically regarding accessor
> functions. which is probably a better design?
>
> class foo
> {
> public:
>     void name(string const& n) { name_ = n; }
>     string name() const { return name_; }
> private:
>     string name_;
> };
>
> class foo
> {
> public:
>     string& name() { return name_; }
>     string name() const { return name_; }
> private:
>     string name_;
> };

class RemoteFoo {
Socket commSocket; // sends and receives data to/from another computer
public:
// wich public interface works here?
void name( string const& n );
// works well we send 'n' to the other computer
string name() const;
// works well we request name from the other computer and return it
string& name();
// no good, we are forced to maintain data on this computer
};

class DatabaseFoo {
DB db; // stores data in a database
public:
// wich public interface works here?
void name( string const& n );
// works well we send 'n' to the Database
string name() const;
// works well we request name from the Database and return it
string& name();
// no good, we are forced to maintain data in RAM
};

My point here is that "string& foo::name()" forces us into a spicific
implementation and that is a Bad Thing.


> the difference from the client point of view is how you set the name:
>
> foo f;
> f.name("name");
> // or
> f.name() = "name";

The difference from the client point of view is neglidgable, the
difference from the server point of view is great.

> are there any thoughts on this?

IMO your best bet is to start with:

class foo {
string name_;
public:
const string name() const { return name_; }
void name( const string& n ) {
name_ = n;
}
};

Then if profiling shows that calling "name() const" is too expensive,
change it to "const string& foo::name() const" wich will not affect
existing clients in any way.

>> foo f; 

Another possible argument in favor of f.name("name") would be that the funct
ion
could perform a sanity check on its argument if necessary.

As another poster pointed out simply returning a reference to a data member,
even one that is part of the visible state of the object, is probably just a
s
bad as having that data member be public.

Mark A. Gibbs wrote:

>
> i have a question about class design, specifically regarding accessor
> functions. which is probably a better design?
>
> class foo
> {
> public:
>    void name(string const& n) { name_ = n; }
>    string name() const { return name_; }
> private:
>    string name_;
> };
>
> class foo
> {
> public:
>    string& name() { return name_; }
>    string name() const { return name_; }
> private:
>    string name_;
> };
[snip]

>
> are there any thoughts on this?
>
> mark
>
>

I would choose to return a _copy_ of the string, not
a reference to the string.  A copy allows the string
to be stored elsewhere or computed on-the-fly.  Passing
by reference means that the string variable must always
exist.

This has bitten me on several occasions. :-(

--
Thomas Matthews

C++ newsgroup welcome message:
http://www.slack.net/~shiva/welcome.txt
C++ Faq: http://www.parashift.com/c++-faq-lite
C Faq:   http://www.eskimo.com/~scs/c-faq/top.html
alt.comp.lang.learn.c-c++ faq:
http://www.comeaucomputing.com/learn/faq/
Other sites:
http://www.josuttis.com  -- C++ STL Library book

On Wed, 18 Aug 2004 14:28:53 GMT, Thomas Matthews
< Thomas_MatthewsSpitsOnSpamBots@sbcglobal
.net> wrote:

[snip]
>I would choose to return a _copy_ of the string, not
>a reference to the string.  A copy allows the string
>to be stored elsewhere or computed on-the-fly.

Well, only if you are paranoid about clients grabbing the address of
the string and doing unmentionable things to it (even if it is a const
&) ...

>Passing by reference means that the string variable must always
>exist.

How could the variable "not exist" if you have a string member? It
might be empty, but it does exist. I can only see a problem if you
have a pointer to a string as member, or perhaps a char *, and try to
dereference it when it is null.

>This has bitten me on several occasions. :-(

How?


--
Bob Hairgrove
NoSpamPlease@Home.com

Bob Hairgrove wrote: 
>
>
> How could the variable "not exist" if you have a string member? It
> might be empty, but it does exist. I can only see a problem if you
> have a pointer to a string as member, or perhaps a char *, and try to
> dereference it when it is null.
>
> 
>
>
> How?

he has a good point actually - imagine i was making a class with a
colour member, and i chose to initially represent that colour as a long:

class foo
{
public:
void set_colour(long c) { c_ = c; }
long const& colour() const ( return c_; }

private:
long c_;
};

then later i changed it to use an existing colour class:

class foo
{
public:
void set_colour(long c) { c_.assign_long(c); c_as_long_ = c; }
long const& colour() const ( return c_as_long_; }

private:
colour_type c_;
long c_as_long_;
};

that's what i'd have to do to maintain the existing interface. on the
other hand, if i'd done:

class foo
{
public:
void set_colour(long c) { c_ = c; }
long colour() const { return c_; }
private:
long c_;
};

then to change c_ to a colour_type is trivial.

class foo
{
public:
void set_colour(long c) { c_.assign_long(c); }
long colour() const { return c_.to_long(); }
private:
colour_type c_;
};

mark

so to summarize several posts (thank you to all contributors), i
personally think the best choice for me would be:

class foo
{
public:
void set_name(string const& n) { n = name_; }
string const& name() const { return name_; }
private:
string name_;
};

daniel t. mentioned that returning references from functions makes using
the function in a distributed environment more difficult, and if the
value returned has to be read from an external source it would have to
have a copy maintained in ram. my response to the first concern is that
i, personally, never write code to be used across application
boundaries, but if i did, "string const& name() const" is functionally
the same as "string name() const", so i don't see why i'd have to change
anything (but if i did, the change does not really affect client code,
beyond making it more expensive). as for the second point, i was
referring specifically to member accessors, but if i were to deal with
the case mentioned, i would think maintaining a local copy of small
amounts of data would probably be "better" performance-wise than reading
it out of a hard disk on each call - but if it cost too much memory to
maintain the data, and/or the data should be read "fresh" on each call,
i would not disguise the call as a call to an accessor, i would use an
imperative function name (as mr. steinbach suggested) and return by
value, so the costs are clearly visible.

i think that in general i can assume a certain amount of trust with
client code. if someone really wants to do dumb things, there are plenty
of ways to do it in c++. in the same way that basic_string.c_str() notes
that the memory pointed to in the return value belongs to the string and
1) should not be changed by the application or 2) may become invalid
after a modifying operation on the string, i think i can note to client
code that the object referenced in the return belongs to the class,
should not be messed around with by clients and may be invalid after a
modifying operation. there's no practical way you can "accidently" do
anything unkosher to a const reference. and maintaining references to
any object without a severely restricted scope is a risky thing to do.

mark

"Mark A. Gibbs" <x_gibbsmark@rogers.com_x> wrote in message news:<rFyUc.1772
127$Ar.1246605@twister01.bloor.is.net.cable.rogers.com>...
[snip]
> class foo
> {
> public:
>     string& name() { return name_; }
>     string name() const { return name_; }
> private:
>     string name_;
> };

Does that compile? The two name functions differ only in the
type of the returned object, one a string, the other a string &.
Socks