accessors

[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";

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

 

[Ron Natalie]

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.

 

[Alf P. Steinbach]

* 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).

 

[Daniel T.]

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.

 

[DaKoadMunky]

foo f;
Another possible argument in favor of f.name("name") would be that the function
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 as
bad as having that data member be public.

 

[Thomas Matthews]

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

 

[Bob Hairgrove]

On Wed, 18 Aug 2004 14:28:53 GMT, Thomas Matthews

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

 

[Mark A. Gibbs]

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

 

[Mark A. Gibbs]

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

 

[puppet_sock]

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

 

[Xenos]

"Mark A. Gibbs" <[email protected]_x> wrote in message

[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

Yes, because they also differ in "constantness."

 

[Thomas Matthews]

On Wed, 18 Aug 2004 14:28:53 GMT, Thomas Matthews

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

Let us take an example from the realm of databases.
Let us have a string field which is a column of
strings in a table.

Now let us have a table of the form:
[integer, string]
Such that there is a 1:1 relationship between the
integer and the string. No duplicate strings or
integers are allowed.

I have a class already called Publisher. It is
a string representing a Publisher's name. Here
is the simple class:
class Publisher /* 1 */
{
public:
string get_name(void) const;
private:
string name;
};

I now want to convert the Publisher class to
be a proxy or look-up field. The publisher
class will change from having a string to
having an integer. The get_name() method
will now use the integer as an index into
a table and retrive the string. The Publisher
class becomes:

class Publisher /* 2 */
{
public:
string get_name(void) const;
private:
int record_index;
};

I have changed the _implementation_ but kept
the interface the same. None of the clients
of this class need to be retested and (in
theory[1]) not recompiled.

Had I chosen to to have the get_name() method
to return a "const string&" then I would have
to have a duplicate or resident string
variable[2] because the "reference" notation means
that the variable must exist. This cause more
code and data space:
class Publisher /* 3 */
{
public:
string get_name(void) const;
private:
int record_index;
string name;
};

An even worse problem is that the "get_name()"
method now modifies the "name" member. So it
isn't "const" anymore; but all the clients
expect it to be. Hmmm, now we get ugly.
Enter now "mutable":
class Publisher /* 4 */
{
public:
string get_name(void) const;
private:
int record_index;
mutable string name;
};

The "mutable" modifier now allows a the constant
method "name" to modify the variable. All this
work, simply to statisfy the requirement of using
a reference and because the notion that using
references is more efficient and leads to cleaner,
more readable code. Malarky! Phooey!.

The burning sensation comes from having to:
1. Maintain a mirror[2] or duplicate data member.
{Don't forget that your new design has to
account for determining when the data member
needs to be refreshed or is invalid.}
2. Apply the "mutable" modifier to constant functions.
This application takes away the primary reason
for having private data members in the first place.

----
[1] I said "in theory" because in most systems,
source files are recompiled when their header
files are changed. The above change did not
change the interface so all clients did not
need to be recompiled. {Similar issue with
adding comments}.

[2] In many instances, data fields are mirrored to
speed up access times. The datum is only fetched
from the table once. Additional accesses return
the mirrored value, thus eliminating extra work of
fetching from table. But if the table changes
without knowledge, the mirror concept causes more
problems (using old and probably invalid data).

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

 

[Alf P. Steinbach]

* Thomas Matthews:

Let us take an example from the realm of databases.
Let us have a string field which is a column of
strings in a table.

Now let us have a table of the form:
[integer, string]
Such that there is a 1:1 relationship between the
integer and the string. No duplicate strings or
integers are allowed.

I have a class already called Publisher. It is
a string representing a Publisher's name. Here
is the simple class:
class Publisher /* 1 */
{
public:
string get_name(void) const;
private:
string name;
};

I now want to convert the Publisher class to
be a proxy or look-up field. The publisher
class will change from having a string to
having an integer. The get_name() method
will now use the integer as an index into
a table and retrive the string. The Publisher
class becomes:

class Publisher /* 2 */
{
public:
string get_name(void) const;
private:
int record_index;
};

I have changed the _implementation_ but kept
the interface the same. None of the clients
of this class need to be retested and (in
theory[1]) not recompiled.

The clients need to be recompiled. They need to be recompiled
because parts that they depend on have been changed. The clients
depend on the member variables because the member variables, even
the 'private' ones, define how much memory each instance requires.

To do what you seem to intend define a pure abstract interface:


class IPublisher
{
public:
virtual std::string name() const = 0;
};


Note (1) use of 'std::', (2) no 'get_', i.e. accessor function named
for what it returns rather than how it does, (3) no C'istic 'void'.

Then you can derive various PublisherWhatever classes from this
interface, and if you do it properly, where client code depends only
on IPublisher, client code will not need recompilation.

However, the example is not relevant to the original problem.