From: Dervish on
Here is simple code piece:

class A
{
unsigned numCalls; //<<---point 1
typedef map<string,string> AMAP;
AMAP aMap;
public:
A() : numCalls(0){}
const string& GetString(const string& s) //<<---point 2
{
numCalls++;
AMAP::const_iterator it = aMap.find(s); //<<---point 3
if (it!=aMap.end())
return it->second;
static const string nothing = "nothing";
return nothing;
}
};

1) Is it a good idea to make GetString const (point 2) by making
numCalls
mutable (point 1)?
2) Is there any sense to replace line at point 3 with following:
const AMAP::const_iterator& it = aMap.find(s);
Note that map::find() returns by value.


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Mike Capp on
> 1) Is it a good idea to make GetString const (point 2) by making
> numCalls mutable (point 1)?

Hard to say since the example code doesn't really make sense. The
'mutable' keyword is for when a const member function touches internal
state, but not "logical" (that is, externally visible) state. numCalls
isn't externally visible, so yes, by that rationale it could be made
mutable and GetString could be made const. As it stands, though,
numCalls isn't used for anything at all, so the obvious "good idea" is
to delete it altogether.

What real usage did you have in mind when asking this question?

cheers
Mike


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Heinz Ozwirk on
"Dervish" <DAkhlynin(a)yandex.ru> schrieb im Newsbeitrag
news:1138192193.094463.24170(a)g43g2000cwa.googlegroups.com...
> Here is simple code piece:
>
> class A
> {
> unsigned numCalls; //<<---point 1
> typedef map<string,string> AMAP;
> AMAP aMap;
> public:
> A() : numCalls(0){}
> const string& GetString(const string& s) //<<---point 2
> {
> numCalls++;
> AMAP::const_iterator it = aMap.find(s); //<<---point 3
> if (it!=aMap.end())
> return it->second;
> static const string nothing = "nothing";
> return nothing;
> }
> };
>
> 1) Is it a good idea to make GetString const (point 2) by making
> numCalls
> mutable (point 1)?

Does changing of numCalls have any visible effect to the outside? Is there
any public (or protected) member function, that returns different values
depending on the value of numCalls? Or do you simply want to count the
number of calls for statistcal or profiling reasons? If changing the value
of numCalls has no visible effect, making GetString const and numCalls
mutable is ok, at least as long as it is never tried to create constant
objects of that type. It is ok to use references of pointers to constant
objects, but all these objects must have been defined as non-constant
objects. If you have a constant object (like 'const A foo;') the compiler
might try place such objects in read-only memory (or is the compiler not
allowd to do such things if a class contains mutable data members?).

> 2) Is there any sense to replace line at point 3 with following:
> const AMAP::const_iterator& it = aMap.find(s);

The result of map<>::find is local to the function that calls map<>::find.
And it is never a good idea to return a reference or pointer to a local
object. So you should declare (and define) GetString as

std::string const& GetString ...

or

AMAP::const_iterator GetString ...

Usually iterators are not that expensive to copy, that returning a reference
is much faster that returning a reference (in many situations, at least).

One final note - if there is no match, I wouldn't return a reference to a
string having a non-empty value. "nothing" might be a valid value in the
map, and it is quite expensive to test against such a value. Usually I would
prefer to return (a reference to) an empty string in such cases.

HTH
Heinz



[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Daniel T. on
In article <1138192193.094463.24170(a)g43g2000cwa.googlegroups.com>,
"Dervish" <DAkhlynin(a)yandex.ru> wrote:

> Here is simple code piece:
>
> class A
> {
> unsigned numCalls; //<<---point 1
> typedef map<string,string> AMAP;
> AMAP aMap;
> public:
> A() : numCalls(0){}
> const string& GetString(const string& s) //<<---point 2
> {
> numCalls++;
> AMAP::const_iterator it = aMap.find(s); //<<---point 3
> if (it!=aMap.end())
> return it->second;
> static const string nothing = "nothing";
> return nothing;
> }
> };
>
> 1) Is it a good idea to make GetString const (point 2) by making
> numCalls
> mutable (point 1)?

As written, 'numCalls' is purely an internal thing that doesn't affect
the visible state of objects of the class at all, so yes, I would make
GetString const and numCalls mutable. However, if there is a public,
getNumCalls member-function, or if some other member-function returns
different values based on the value of numCalls, then I would leave it
as it is.

> 2) Is there any sense to replace line at point 3 with following:
> const AMAP::const_iterator& it = aMap.find(s);
> Note that map::find() returns by value.

I'll leave this one for someone more skilled to answer.


--
Magic depends on tradition and belief. It does not welcome observation,
nor does it profit by experiment. On the other hand, science is based
on experience; it is open to correction by observation and experiment.

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Carlos Moreno on
Heinz Ozwirk wrote:

> [...]
>
> Does changing of numCalls have any visible effect to the outside? Is there
> any public (or protected) member function, that returns different values
> depending on the value of numCalls? Or do you simply want to count the
> number of calls for statistcal or profiling reasons?

Or for cache (optimization) purposes. Though in this case, numCalls
does not sound like a "cached" value -- but imagine you have a class
for a polygon of some sort, and you have a member that returns the
area; if computation of it is expensive, you could setup a data
member that holds the "pre-computed" area and assign it only on
demand; in such case, you could assign a negative value to that
data member, and the first time it is called (through a *const*
member function area()), you store the result in the *mutable* data
member cached_area, so that the next time someone calls area() you
don't need to re-compute it.

Carlos
--

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]