operator[] -- returning references

Discussion in 'C++' started by cppaddict, Jun 11, 2004.

  1. cppaddict

    cppaddict Guest

    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.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
    cppaddict, Jun 11, 2004
    #1
    1. Advertising

  2. cppaddict

    Rolf Magnus Guest

    cppaddict wrote:

    > Hi,
    >
    > Is it considered bad form to have the subscript operator return a
    > const reference variable?


    No, not for the const version of it. It's actually the preferred way.

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


    Throw an exception.
    Rolf Magnus, Jun 11, 2004
    #2
    1. Advertising

  3. cppaddict

    cppaddict Guest


    >> Does anyone have any thoughts/suggestions?

    >
    >Throw an exception.


    Rolf,

    Doesn't the compiler require a return statement no matter what? It
    seems to for me. Am I missing something?

    Thanks,
    cpp
    cppaddict, Jun 11, 2004
    #3
  4. cppaddict

    Rolf Magnus Guest

    cppaddict wrote:

    >
    >>> Does anyone have any thoughts/suggestions?

    >>
    >>Throw an exception.

    >
    > Rolf,
    >
    > Doesn't the compiler require a return statement no matter what?


    Not if that code path can't be reached anyway. Actually, my compiler.
    You could just write:

    const MyClass& SubscriptTest::lookUpByChar(char ch) const {
            for (unsigned i=0; i<_vect.size(); i++) {
                    if (_vect.getChar() == ch)
                            return i;
            }
            
    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.
    Rolf Magnus, Jun 11, 2004
    #4
  5. cppaddict

    Rolf Magnus Guest

    Rolf Magnus wrote:

    > Not if that code path can't be reached anyway. Actually, my compiler.


    Insert "gives a warning if there is a return statement that can never be
    reached" at a place of your choice.
    Rolf Magnus, Jun 11, 2004
    #5
  6. cppaddict

    Daniel T. Guest

    In article <>,
    cppaddict <> wrote:

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


    The const version and the non-const version of op[] should do the same
    thing if the item isn't in the container.

    You have a few choices, declare the behavior undefined (thats how op[]
    is defined in vector,) insert an element (the way op[] works in map,) or
    throw an exception (like vector::at does.)

    What is best for the code that uses your container?
    Daniel T., Jun 11, 2004
    #6
  7. cppaddict

    AngleWyrm Guest

    "Rolf Magnus" <> wrote in message
    news:cac1un$jgs$02$-online.com...

    > 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.
    AngleWyrm, Jun 11, 2004
    #7
  8. Rolf Magnus wrote:

    > cppaddict wrote:
    >
    >> Hi,
    >>
    >> Is it considered bad form to have the subscript operator return a
    >> const reference variable?

    >
    > No, not for the const version of it. It's actually the preferred way.


    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 Yegorushkin
    Maxim Yegorushkin, Jun 11, 2004
    #8
  9. cppaddict

    cppaddict Guest

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

    >You have a few choices, declare the behavior undefined (thats how op[]
    >is defined in vector,)


    How do you declare behaviour as undefined?

    Thanks,
    cpp
    cppaddict, Jun 11, 2004
    #9
  10. cppaddict

    cppaddict Guest

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

    Thanks,
    cpp
    cppaddict, Jun 11, 2004
    #10
  11. cppaddict

    Rolf Magnus Guest

    AngleWyrm wrote:

    > "Rolf Magnus" <> wrote in message
    > news:cac1un$jgs$02$-online.com...
    >
    >> 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.
    Rolf Magnus, Jun 11, 2004
    #11
  12. cppaddict

    bartek Guest

    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
    bartek, Jun 11, 2004
    #12
  13. cppaddict

    Daniel T. Guest

    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.
    Daniel T., Jun 11, 2004
    #13
  14. cppaddict

    cppaddict Guest


    >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, Jun 11, 2004
    #14
  15. "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.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::eek:perator[]

    const MyString s2 = "Bar";
    cout << s2[0]; // calls const MyString::eek: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 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
    Dave Townsend, Jun 11, 2004
    #15
  16. cppaddict

    Daniel T. Guest

    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.
    Daniel T., Jun 11, 2004
    #16
  17. cppaddict

    cppaddict Guest


    >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, Jun 11, 2004
    #17
  18. 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
    Maxim Yegorushkin, Jun 12, 2004
    #18
  19. cppaddict

    Siemel Naran Guest

    "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.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.getChar() == ch) break;
    }
    if (i == _vect.size()) throw SomeException();
    return _vect;
    }


    > 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.
    Siemel Naran, Jun 12, 2004
    #19
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Roger Leigh
    Replies:
    8
    Views:
    413
    Karl Heinz Buchegger
    Nov 17, 2003
  2. Replies:
    3
    Views:
    432
    Victor Bazarov
    Nov 10, 2004
  3. DanielEKFA
    Replies:
    8
    Views:
    584
    DanielEKFA
    May 16, 2005
  4. Replies:
    8
    Views:
    692
    Bruno Desthuilliers
    Dec 12, 2006
  5. Lars Willich
    Replies:
    13
    Views:
    819
    Ian Shef
    Oct 23, 2007
Loading...

Share This Page