«

AngleWyrm wrote:

> "Rolf Magnus" <> wrote in message
> news:cac1un$jgs$02$...
>
>> throw SomeException("character not found");
>> return MyClass(); //shut the compiler up
>> }
>
>> Yes, this would mean reuturning a reference to a temporary, but
> since
>> it's never actually reached, it doesn't hurt.
>
> Isn't there a #pragma to disable specific warnings for one or more
> instances/lines? I've never used pragma, but this might be a good
> place for it, if that's how it's used.

There might be, but it depends entirely on the compiler. The C++
standard doesn't say anything about which pragmas exist or what they
do.

cppaddict <> wrote in
news::

>>The const version and the non-const version of op[] should do the same
>>thing if the item isn't in the container.
>
> In my case, wouldn't it violate encapsulation to have a non-const
> version? Essentially, that would be returning a refernece to private
> data.

This is a question of design, and of your goals.
Consider the std::vector container - there is a non-const operator[]
overload provided, because the role of the container is to *hold*
objects, not *hide* them.

So, if your class represents some form of a container, therefore what's
the point of not providing operator[] that returns a non-const reference?

>
>>You have a few choices, declare the behavior undefined (thats how op[]
>>is defined in vector,)
>
> How do you declare behaviour as undefined?

You basically state it clearly in the documentation.

--
:: bartekd [at] o2 [dot] pl

In article <>,
cppaddict <> wrote:

>>The const version and the non-const version of op[] should do the same
>>thing if the item isn't in the container.
>
>In my case, wouldn't it violate encapsulation to have a non-const
>version? Essentially, that would be returning a refernece to private
>data.

Sorry, I thought this was simply some sort of container.

(Warning, the code below uses 'compose1' which was origionally part of
the STL but never actually made it into the standard.)

The proper way to do what it is you were attemptin is:

class SubscriptTest {
static const MyClass _default; // *** notice me
typedef std::vector<MyClass> MyClassContainer;
MyClassContainer _myClasses;
public:
const MyClass& operator[](char ch) const { return lookUpByChar(ch); }
const MyClass& lookUpByChar(char ch) const {
using namespace std;
MyClassContainer::const_iterator it =
find_if(_myClasses.begin(), _myClasses.end(),
compose1(bind2nd(equal_to<char>(), ch),
mem_fun_ref(&MyClass::getChar)));
return it == _myClasses.end() ? _default : *it; // ** notice me
}
};

The idea here is that you have a static const MyClass object "_default"
that is returned whenever lookUpByChar can't find an approprate object.
It works like your origional did, except without the memory allocation
hassles.

The problem is, the client won't like the result:

void client( const SubscriptTest& st ) {
assert( st['a'].getChar() == 'a' );
}

The assert will fail if the default object is returned.

So despite the fact that it "works", it's not a good idea.


>>You have a few choices, declare the behavior undefined (thats how op[]
>>is defined in vector,)
>
>How do you declare behaviour as undefined?

First you would need a sanity check:

// return true if a MyClass object can be
// found who's getChar() returns ch
bool SubscriptTest::hasMyClass( char ch ) const {
return find_if( _myClasses.begin(),
_myClasses.end(), compose1( bind2nd( equal_to<char>(), ch ),
mem_fun_ref(&MyClass::getChar) ) ) != _myClasses.end();
}

then:

// if 'ch' can't be found, then results are undefined
const MyClass& SubscriptTest::lookUpByChar(char ch) const {
assert( hasMyClass( ch ) ); // *** notice me
return find_if( _myClasses.begin(),
_myClasses.end(), compose1( bind2nd( equal_to<char>(), ch ),
mem_fun_ref(&MyClass::getChar) ) );
}

Here, if 'ch' isn't found, the function returns *_myClasses.end() which
is of course, undefined. Fortunatly, in debug mode we first check to
make sure that the MyClass object exists. In release, the client using
this code will just have to be careful.

>The idea here is that you have a static const MyClass object "_default"
>that is returned whenever lookUpByChar can't find an approprate object.
>It works like your origional did, except without the memory allocation
>hassles.
>
>The problem is, the client won't like the result:
>
>void client( const SubscriptTest& st ) {
> assert( st['a'].getChar() == 'a' );
>}
>
>The assert will fail if the default object is returned.
>
>So despite the fact that it "works", it's not a good idea.

Daniel,

I don't understand why the client won't like the result when the
_default object is returned. Could you explain further?

Thanks,
cpp

"cppaddict" <> wrote in message
news:...
> Hi,
>
> Is it considered bad form to have the subscript operator return a
> const reference variable? If not, what is the proper way to do it?
> My question was prompted by the code below, my problematic attempt to
> implement a subscript operator that returns a const reference. The
> dubious code is marked at the end.
>
> <code>
>
> //MyClass is a large class we don't want to copy alot
> //MyClass has a "char getChar()" function
> class MyClass;
>
> class SubscriptTest {
> private:
> std::vector<MyClass> _vect;
> public:
> SubscriptTest() {};
> void addObj(MyClass mc) {_vect.push_back(mc);}
> const MyClass& operator[] (char ch) const {return
> lookUpByChar(ch);}
> };
>
> const MyClass& SubscriptTest::lookUpByChar(char ch) const {
> for (unsigned i=0; i<_vect.size(); i++) {
> if (_vect[i].getChar() == ch)
> return i;
> }
>
> //PROBLEM IS HERE
> return *(new MyClass); //return value here needed to compile
> }
>
> </code>
>
> The problem is what to return when the lookup fails. Creating a
> default value via new, as I do above, seems wrong -- it wastes memory
> and requires tracking the objects created and then destroying them in
> Subscript's destructor.
>
> Another option would be to create the default value as static variable
> inside the lookUpByChar function or as a member variable of
> SubscriptTest, and have the function always return a reference to
> that. That seems reasonable to me, but I still wasn't sure.
>
> Does anyone have any thoughts/suggestions?
>
> Thanks,
> cpp
>

CPP,

Returning a const reference is not only good form, but is the right thing to
do if the
circumstances demand it. In the situation where you are invoking the
subscript operator
on a const object, you might want to return a const reference. In fact, you
probably need
two implementations, non-const and const so you can modify values and just
read them
respectively.

For example, in this example, the second call to operator[] uses a const
reference since s2 is a const object.
If you step through this code in the debugger, you will see the correct flow
through both the const and non-const operator[], and the appropriate
operator[] for the
element access within the _data vectors of MyString.

#include <vector>
#include <iostream>
using namespace std;

class MyString {
public:

MyString( char * data )
{ _data.assign( data, data+strlen(data)+1); }

char& operator[](int position) { return _data[position]; }

const char& operator[](int position) const { return _data[position]; }

private:
vector<char> _data;
};


int main()
{
MyString s1 = "Foo";
cout << s1[0]; // calls non-const MyString:perator[]

const MyString s2 = "Bar";
cout << s2[0]; // calls const MyString:perator[]

return 0;
}

I have a second point. In your posting, you say that the Myclass objects
are expensive/large to
create and so you don't want to create them unnecessarily. If your
operator[] returns references,
then you do not have to create any new objects, you can do something like
this:


class Dictionary
{
private:
vector<MyString> _words;
public:
Dictionary() {}

~Dictionary() {}

MyString& operator[]( int i )
{

int x = 0;
for ( vector<MyString>::iterator it=_words.begin(); it!=_words.end();
++it, ++x)
{
if ( i == x)
{
return *it;
}
}
// throw - you should never reach here...
// return statement here to defeat compiler error/ warning.
return _words[0];
}
const MyString& operator[] (int i ) const
{

int x =0;
for ( vector<MyString>::const_iterator cit=_words.begin();
cit!=_words.end(); ++cit, ++x)
{
if ( i == x)
{
return *cit;
}
}
// throw - we should not ever get here if i is a valid index. ....

// return statement here to defeat compiler error/warning.
return _words[0];
}


};

this example is a bit contrived, but it meant to illustrate the point that
you can avoid the creation
of new objects by returning references to existing ones in _words vector,
either the one you
are looking for _words[i] or a default value _words[0] in case of failure.
This satisfies the
need for the compiler to find a return through all branches of the function
and stylistic needs too.


Finally, the operator[] should throw an exception in the case of an invalid
indexing value ( well
at least in the "read" case), there is not much else it can reasonably do!
It is not a reasonable interpretation to use operator[] to do lookup of an
element, that should be another function with a different interface suitable
for the task.

dave

In article <>,
cppaddict <> wrote:

>>The idea here is that you have a static const MyClass object "_default"
>>that is returned whenever lookUpByChar can't find an approprate object.
>>It works like your origional did, except without the memory allocation
>>hassles.
>>
>>The problem is, the client won't like the result:
>>
>>void client( const SubscriptTest& st ) {
>> assert( st['a'].getChar() == 'a' );
>>}
>>
>>The assert will fail if the default object is returned.
>>
>>So despite the fact that it "works", it's not a good idea.
>
>Daniel,
>
>I don't understand why the client won't like the result when the
>_default object is returned. Could you explain further?

I showed why. When the client calls:

char ch = mySubscriptTest['a'].getChar();

The client has every right to expect ch to equal 'a'. Unless you can
somehow guaruntee that it will, the client will be unhappy. To make such
a guaruntee, you can't use a system where a default object is returned.

>I showed why. When the client calls:
>
>char ch = mySubscriptTest['a'].getChar();
>
>The client has every right to expect ch to equal 'a'. Unless you can
>somehow guaruntee that it will, the client will be unhappy. To make such
>a guaruntee, you can't use a system where a default object is returned.

Ok. That makes sense. Thanks for all explanations,

cpp

cppaddict <> wrote:

>> This is rather arguable and senseless outside a context.
>>
>> There is a rule of thumb: when in doubt do what built-in types do. Consider the operator[]'s behavior applied to a pointer - it always returns a pointee type reference, no matter whether the pointer itself is cv-qualified or not.
>>
>> char c(0);
>> char* p1 = &c; // non-const pointer, non-const pointee
>> char* const p2 = &c; // const pointer, non-const pointee
>>
>> char& r1 = *p1;
>> char& r2 = *p2;
>
> Maxim,
>
> It's not clear to me what point you are trying to make or what advice
> you are trying to give with this example. Could you please explain?

Sorry for being not clear. My aforementioned point was an example of an argument.

I think that the return type for UDT's operator[] must be decided on a per class basis. And there is no way to tell what in general operator[] must return.

Does my point make some sence?

--
Maxim Yegorushkin

"Rolf Magnus" <> wrote in message
news:cac1un$jgs$02$-
> cppaddict wrote:

> >>Throw an exception.

You can also return a pointer, where NULL means no object to return. I
think the use of returning references and using exceptions might lead to
code that is more error-free, because people may forget to test the returned
pointer against NULL.


> const MyClass& SubscriptTest::lookUpByChar(char ch) const {
> for (unsigned i=0; i<_vect.size(); i++) {
> if (_vect[i].getChar() == ch)
> return i;
> }
>
> throw SomeException("character not found");
> return MyClass(); //shut the compiler up
> }

I'd write that as

const MyClass& SubscriptTest::lookUpByChar(char ch) const {
unsigned i = 0;
for ( ; i<_vect.size(); i++) {
if (_vect[i].getChar() == ch) break;
}
if (i == _vect.size()) throw SomeException();
return _vect[i];
}


> Yes, this would mean reuturning a reference to a temporary, but since
> it's never actually reached, it doesn't hurt.

True indeed, though code like returning a reference to a temporary tends to
raise warning bells.