hasMember

[Andrea Crotti]

Suppose I have a class like this

class Cont
{
private:
std::map<int, int> content;

bool hasMember(int idx);
int getValue(int idx);
};

Now what I'm doing now in cases like this is to make sure that every
time I call getValue I'm sure that the object is there, so I have to
call first hasMember.

In the caller I always do

if (obj.hasMember(idx))
int idx = obj.getValue(idx);


in this way I don't need to handle special cases when the thing is not
found, BUT in many cases this brings to useless computations, since
sometimes I have to scan the container twice.

Is there a better pattern in these situations which is not "return -1
if the object is not found"?

 

[Michael Doubez]

Suppose I have a class like this

class Cont
{
private:
        std::map<int, int> content;

        bool hasMember(int idx);
        int getValue(int idx);

};

Now what I'm doing now in cases like this is to make sure that every
time I call getValue I'm sure that the object is there, so I have to
call first hasMember.

In the caller I always do

if (obj.hasMember(idx))
   int idx = obj.getValue(idx);

I assume you don't really want to use an uninitialized idx but want to
update idx value.

in this way I don't need to handle special cases when the thing is not
found, BUT in many cases this brings to useless computations, since
sometimes I have to scan the container twice.

Is there a better pattern in these situations which is not "return -1
if the object is not found"?

I don't know of pattern specifically.
Depending on your need, you can
* use something like Barton-Nackman's fallible<T> (this is the most
general solution):
// fallible<int> getValue(int idx);
fallible<int> idx = obj.getValue(n)
if( idx.isSet() ) ...
* use a default value (useful if you only want ot update a value),
maybe with an optional parameter telling you if the value is set or if
the default value was used:
// int getValue(int idx, int defaultValue, bool* isSet=0);
int idx = 42;
bool isSet;
idx = obj.getValue(idx,idx,&isSet);
if( isSet ) ...
* return a pointer (this is the most lightweight if you have an
adequate storage of the values to return).
// int const * getValue(int)
int const *idx = obj.getValue(n);
if( idx ) ...

There are other solutions but you would have to be more specific.

 

[James Kanze]

Suppose I have a class like this

class Cont
{
private:
std::map<int, int> content;

bool hasMember(int idx);
int getValue(int idx);
};

Now what I'm doing now in cases like this is to make sure that every
time I call getValue I'm sure that the object is there, so I have to
call first hasMember.

In the caller I always do

if (obj.hasMember(idx))
int idx = obj.getValue(idx);

in this way I don't need to handle special cases when the thing is not
found, BUT in many cases this brings to useless computations, since
sometimes I have to scan the container twice.

Is there a better pattern in these situations which is not "return -1
if the object is not found"?

The simplest solution is to return a pointer to the map's
contents, or NULL if the value isn't there, e.g.:

int const* Cont::getValue(int idx) const
{
std::map<int, int>::const_iterator result
= content.find(idx);
return result == content.end()
? NULL
: &result->second;
}

and:

int const* i = obj.getValue(idx);
if ( i != NULL ) {
...
}

 

[Andrea Crotti]

The simplest solution is to return a pointer to the map's
contents, or NULL if the value isn't there, e.g.:

int const* Cont::getValue(int idx) const
{
std::map<int, int>::const_iterator result
= content.find(idx);
return result == content.end()
? NULL
: &result->second;
}

and:

int const* i = obj.getValue(idx);
if ( i != NULL ) {
...
}

I would do that in C but I don't think it looks nice in C++...

Using exceptions in this case could be good?

 

[Jeff Flinn]

I would do that in C but I don't think it looks nice in C++...

To avoid the memory allocation and ownership issues use boost:ptional.

#include <boost/optional.hpp>

typedef boost:ptional<int> optional_int;

optional_int Cont::getValue(int idx) const
{
std::map<int, int>::const_iterator itr = content.find(idx);

return (itr != cont.end())? itr->second : optional_int();
}

and:

if(optional_int i = obj.getValue(idx)) // safe bool idiom
{
int x = *i + 123; // deref for value
}

Using exceptions in this case could be good?

Seems out of place for the use you've described, given what little info
you've provided.

Jeff

 

[Andrea Crotti]

To avoid the memory allocation and ownership issues use boost:ptional.

#include <boost/optional.hpp>

typedef boost:ptional<int> optional_int;

optional_int Cont::getValue(int idx) const
{
std::map<int, int>::const_iterator itr = content.find(idx);

return (itr != cont.end())? itr->second : optional_int();
}

and:

if(optional_int i = obj.getValue(idx)) // safe bool idiom
{
int x = *i + 123; // deref for value
}


Seems out of place for the use you've described, given what little
info you've provided.

Jeff

Interesting the optional_int, but I can't use boost...
Anyway I didn't specify much because is a general (for me) problem,
every time I have a class which has some structure (set/vector/map) I
might want to check and get some values from it.

So returning -1 or NULL is for me not nice, using exceptions I meant
something like done in std::vector::at for example

try {
int x = vec.at(0);
} catch ...

I could throw an exception whenever I don't find the value and catch it
in the caller, that's what I meant..

 

[Jonathan Lee]

I would do that in C but I don't think it looks nice in C++...

Would it be more acceptable with a typedef?

class Cont {
public:
typedef const int* const_iterator;
const_iterator end() const { return NULL; }
const_iterator getValue(int idx) { ... }
};

....

Cont::const_iterator it = obj.getValue(idx);
if (it != cont.end()) {
...
}

--Jonathan

 

[Jonathan Lee]

Would it be more acceptable with a typedef?

class Cont {
  public:
    typedef const int* const_iterator;
    const_iterator end() const { return NULL; }
    const_iterator getValue(int idx) { ... }

};

...

Cont::const_iterator it = obj.getValue(idx);
if (it != cont.end()) {
   ...

}

--Jonathan

Though, since const int* has arithmetic operators defined on it
you may not to use the bare pointer. Anyway, the point is that
this really isn't that unusual in C++. The iterator for
std::vector is probably a simple pointer, hidden with a typedef.

--Jonathan

 

[Andrea Crotti]

Anyway I didn't specify much because is a general (for me) problem,

This might reflect a deeper design problem. A class is meant for
encapsulating some bunch of data together with functions operating on
this data. From your description it sounds like trying to tear this
encapsulation apart.

Yes sure, in theory I perfectly agree.
But in my case I have a main actor which has to take the decisions and
(most of) the other classes are actually data containers.

I encapsulate everything I can and expose only the minimum, but I often
need to get values from there.

In my opinion getting something only if hasMember is true was also a way
to encapsulate, the only drawback is sometimes performance.

Exceptions are meant more for propagating errors multiple layers up the
call stack. If the not-found event is common and expected, then it is not
"exceptional" and should not be expressed as an exception. Besides, the
exception mechanism may easily be much slower than searching the
container twice (which was your original worry).

Ok I see, well in other languages is a more common pattern, maybe not
with dict/lists, but in python trying to get something is quite common.

And I don't think it could not be used in C++, otherwise why
std::vector::at uses exactly this mechanism?

Have you considered the simple way of having another output parameter in
addition to the return value?

bool Cont::getValue(int idx, int& value_out) const
{
std::map<int, int>::const_iterator itr = content.find(idx);
if (itr != content.end()) {
value_out = itr->second;
return true;
} else {
return false;
}
}

// ...

int x;
if (obj.getValue(idx, x)) {
// x found and valid, do something with x
}

This has the drawback that the lexical scope of x is too broad, but if
this is a very low-level class used only by a couple of slightly higher-
level classes this might be OK.

hth
Paavo

That also works thanks...
Well at the moment I'll leave everything like this until I'm not sure
about another solution, optional_int would be the cleanest probably.

 

[James Kanze]

I would do that in C but I don't think it looks nice in C++...

Why not?

Using exceptions in this case could be good?

No.

 

[James Kanze]

To avoid the memory allocation and ownership issues use boost:ptional.

There aren't any memory allocation and ownership issues. I use
the Fallible idiom (on which boost/optional is based) a lot, but
in this case, a pointer seems simpler and more appropriate:
a pointer is the standard idiom for a fallible reference.

#include <boost/optional.hpp>

typedef boost:ptional<int> optional_int;

optional_int Cont::getValue(int idx) const
{
std::map<int, int>::const_iterator itr = content.find(idx);
return (itr != cont.end())? itr->second : optional_int();
}

if(optional_int i = obj.getValue(idx)) // safe bool idiom
{
int x = *i + 123; // deref for value
}

Which doesn't work in general, since the returned value (if
found) is supposed to be a reference to the element, not a copy
of it.

I don't know if boost would support boost:ptional<int&>. When
I wrote my own Fallible, it didn't even try to support it,
since you already have fallible references, in the form of
pointers.

 

[James Kanze]

I'm sure boost:ptional is quite easy (and instructive) to implement by
yourself.

Except that he really needs optional<int&>. Which in turn means
partial specialization for reference types, and the partial
specialization would be nothing more than a wrapper around
a pointer. There's really no point in not using a pointer to
begin with.

[...]

Have you considered the simple way of having another output parameter in
addition to the return value?

bool Cont::getValue(int idx, int& value_out) const
{
std::map<int, int>::const_iterator itr = content.find(idx);
if (itr != content.end()) {
value_out = itr->second;
return true;
} else {
return false;
}
}

// ...

int x;
if (obj.getValue(idx, x)) {
// x found and valid, do something with x
}

This has the drawback that the lexical scope of x is too broad, but if
this is a very low-level class used only by a couple of slightly higher-
level classes this might be OK.

If his functions are reasonably small, the increased lexical
scope is not a problem. Defining an uninitialized int is more
bothersome.

But what's wrong with the pointer solution? That's what I'd
like to know. It's certainly the most idiomatic solution for
this problem.

 

[James Kanze]

IMHO, that's worse, since the name "iterator" suggests that if
it isn't end, you can increment it to get the next element.

Though, since const int* has arithmetic operators defined on it
you may not to use the bare pointer. Anyway, the point is that
this really isn't that unusual in C++. The iterator for
std::vector is probably a simple pointer, hidden with a typedef.

Only in the poorer implementations.

In idiomatic C++, one expects to be able to increment (and maybe
decrement) iterators. One does *not* expect this of pointers
(since most pointers point to single objects, e.g. the result of
new).

 

[Jeff Flinn]

There aren't any memory allocation and ownership issues. I use
the Fallible idiom (on which boost/optional is based) a lot, but
in this case, a pointer seems simpler and more appropriate:
a pointer is the standard idiom for a fallible reference.




Which doesn't work in general, since the returned value (if
found) is supposed to be a reference to the element, not a copy
of it.

Andrea's original posting was returning by value. That's what I addressed.

I don't know if boost would support boost:ptional<int&>. When

IIRC, it does.

 

[Jeff Flinn]

Except that he really needs optional<int&>. Which in turn means

Where did this requirement come from?

partial specialization for reference types, and the partial
specialization would be nothing more than a wrapper around
a pointer. There's really no point in not using a pointer to
begin with.

Yes, your right.

[...]

Have you considered the simple way of having another output parameter in
addition to the return value?

bool Cont::getValue(int idx, int& value_out) const
{
std::map<int, int>::const_iterator itr = content.find(idx);
if (itr != content.end()) {
value_out = itr->second;
return true;
} else {
return false;
}
}

// ...

int x;
if (obj.getValue(idx, x)) {
// x found and valid, do something with x
}

This has the drawback that the lexical scope of x is too broad, but if
this is a very low-level class used only by a couple of slightly higher-
level classes this might be OK.

If his functions are reasonably small, the increased lexical
scope is not a problem. Defining an uninitialized int is more
bothersome.

But what's wrong with the pointer solution? That's what I'd
like to know. It's certainly the most idiomatic solution for
this problem.

Agreed.

Jeff

 

[Jonathan Lee]

it isn't end, you can increment it to get the next element.

He's indexing by int... "next" (and "prev") are probably
well defined if he wants to make an actual iterator
out of it. Then he can be somewhat in line with the
STL containers.

Alternatively, he could return
std:air<bool, int>
where the bool indicates success, or
std:air<int, int>
where the first int matches the one he passed in (on
success), etc.

Only in the poorer implementations.

Meaning that it could be done, which is the point I'm
making. He said he didn't want to do a "C style" solution,
but he may very well have used such C style solutions w/
some implementation of vector::iterator and not known it.

--Jonathan

 

[James Kanze]

Where did this requirement come from?

The fact that he's implemention a (map-like) container.