Style: use of member references in derived classes?

Discussion in 'C++' started by Andy Lomax, Jun 24, 2005.

  1. Andy Lomax

    Andy Lomax Guest

    I'm deriving from a base class in a library, and the library vendor is
    expecting me to add a 'string&' member in my derived class. Or, to be
    more precise, they have provided two virtual functions:

    1 void setstr(const std::string& str)
    2 std::string& getstr(void)

    So my obvious implementation is:

    class foo : vendor_bar {
    string& str;
    public:
    virtual void setstr(const string& str_) { str = str_; }
    virtual string& getstr(void) { return str; }
    };

    I've got a bad feeling about this. Is it a good idea to have a
    reference member like this? I can't think of any specific problems,
    but I'd rather use a smart pointer of some sort.

    Another factor is that the string is actually originally provided by
    me (the library user), isn't used elsewhere in the library, and simply
    comes back to me in the 'setstr' call; I use it myself further down in
    the hierarchy. Given this, is there some better way that the library
    could have been designed, rather than hard-wiring in a string
    implementation?

    Cheers

    AL
     
    Andy Lomax, Jun 24, 2005
    #1
    1. Advertising

  2. * Andy Lomax:
    > I'm deriving from a base class in a library, and the library vendor is
    > expecting me to add a 'string&' member in my derived class. Or, to be
    > more precise, they have provided two virtual functions:
    >
    > 1 void setstr(const std::string& str)
    > 2 std::string& getstr(void)


    Bad design.


    > So my obvious implementation is:
    >
    > class foo : vendor_bar {
    > string& str;
    > public:
    > virtual void setstr(const string& str_) { str = str_; }
    > virtual string& getstr(void) { return str; }
    > };


    Change that member declaration to just

    string str;

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Jun 24, 2005
    #2
    1. Advertising

  3. Andy Lomax

    Andy Lomax Guest

    On Fri, 24 Jun 2005 10:07:50 GMT, (Alf P. Steinbach)
    wrote:

    >* Andy Lomax:
    >> I'm deriving from a base class in a library, and the library vendor is
    >> expecting me to add a 'string&' member in my derived class. Or, to be
    >> more precise, they have provided two virtual functions:
    >>
    >> 1 void setstr(const std::string& str)
    >> 2 std::string& getstr(void)

    >
    >Bad design.


    Do you have any specific reasons for saying that it's bad?

    >> So my obvious implementation is:
    >>
    >> class foo : vendor_bar {
    >> string& str;
    >> public:
    >> virtual void setstr(const string& str_) { str = str_; }
    >> virtual string& getstr(void) { return str; }
    >> };

    >
    >Change that member declaration to just
    >
    > string str;


    Hmm... problem is, this class may be instantiated thousands of times,
    all containing exactly the same string. Any other ideas?

    Thanks -

    AL
     
    Andy Lomax, Jun 24, 2005
    #3
  4. Andy Lomax

    John Carson Guest

    "Andy Lomax" <abuse@[127.0.0.1]> wrote in message
    news:
    > On Fri, 24 Jun 2005 10:07:50 GMT, (Alf P. Steinbach)
    > wrote:
    >
    >> * Andy Lomax:
    >>> I'm deriving from a base class in a library, and the library vendor
    >>> is expecting me to add a 'string&' member in my derived class. Or,
    >>> to be more precise, they have provided two virtual functions:
    >>>
    >>> 1 void setstr(const std::string& str)
    >>> 2 std::string& getstr(void)

    >>
    >> Bad design.

    >
    > Do you have any specific reasons for saying that it's bad?
    >
    >>> So my obvious implementation is:
    >>>
    >>> class foo : vendor_bar {
    >>> string& str;
    >>> public:
    >>> virtual void setstr(const string& str_) { str = str_; }
    >>> virtual string& getstr(void) { return str; }
    >>> };

    >>
    >> Change that member declaration to just
    >>
    >> string str;

    >
    > Hmm... problem is, this class may be instantiated thousands of times,
    > all containing exactly the same string. Any other ideas?


    If you only want one string for the whole class, then the obvious thing to
    do is make it a static member.

    --
    John Carson
     
    John Carson, Jun 24, 2005
    #4
  5. Andy Lomax

    Andy Lomax Guest

    On Fri, 24 Jun 2005 22:10:22 +1000, "John Carson"
    <> wrote:

    >If you only want one string for the whole class, then the obvious thing to
    >do is make it a static member.


    Sorry, not making myself clear. The class may be instantiated
    thousands of times, but there may be only 4 or 5 unique values for the
    string; I can't tell before runtime.

    Ideally, I'd like to declare a smart pointer, but I can't see how to
    tie that in with the prototypes that the library vendor has given me.
    I can create a smart ptr for the string when I first create the
    string, before passing it into the library as a reference. However,
    when the library eventually calls 'setstr' with this reference, I
    can't turn it back into the original smart ptr without some sort of
    search.

    Cheers

    AL
     
    Andy Lomax, Jun 24, 2005
    #5
  6. Andy Lomax

    Shaun Guest

    > I'm deriving from a base class in a library, and the library vendor is
    > expecting me to add a 'string&' member in my derived class. Or, to be
    > more precise, they have provided two virtual functions:


    > 1 void setstr(const std::string& str)
    > 2 std::string& getstr(void)


    >Bad design.


    These are NOT badly designed methods. They are, in fact, recommended.
    Rather than rant on why, I'll point you to Herb Sutter's "Exceptional
    C++" series.

    I DO agree, however, that you just want to declare for your member:
    string str;

    The reason is that you'll be taking, in the case of the setter, a const
    reference. The strings copy contructor will not be called and will
    therefore cause undue overhead during the assignment operation.
    Likewise for the getter; you'll return a reference (maybe should be a
    const ref) and avoid copy contruction on the return from the method.
     
    Shaun, Jun 24, 2005
    #6
  7. Andy Lomax

    Shaun Guest

    That should have read, "not cause undue overhead." I apologize for the
    confusion.
     
    Shaun, Jun 24, 2005
    #7
  8. Andy Lomax

    Shezan Baig Guest

    Andy Lomax wrote:
    > I'm deriving from a base class in a library, and the library vendor is
    > expecting me to add a 'string&' member in my derived class. Or, to be
    > more precise, they have provided two virtual functions:
    >
    > 1 void setstr(const std::string& str)
    > 2 std::string& getstr(void)
    >
    > So my obvious implementation is:
    >
    > class foo : vendor_bar {
    > string& str;
    > public:
    > virtual void setstr(const string& str_) { str = str_; }
    > virtual string& getstr(void) { return str; }
    > };
    >



    This shouldnt even compile! What does 'str' refer to?
     
    Shezan Baig, Jun 24, 2005
    #8
  9. Andy Lomax

    Shaun Guest

    You're absolutely correct -- if you assume that the constructor didn't
    assign (and take) a std::string& for an argument. It will compile,
    however, if the class is defined like so:

    ....

    class SomeStrClass
    {
    private:
    std::string& m_str;
    public:
    SomeStrClass(std::string& init_str)
    : m_str(init_str)
    {
    }

    ~SomeStrClass(void)
    {
    }

    void set_str(const std::string& str)
    {
    m_str = str;
    }

    const std::string& get_str(void) const
    {
    return m_str;
    }
    };


    Note that changing the constructor to the following will NOT work:

    SomeStrClass(void)
    : m_str("")
    {
    }

    The reason (if its not obvious) is that you cannot assign a reference
    to a temporary. You'd end up with a "what does this string refer to"
    type situation like your question asks.
     
    Shaun, Jun 24, 2005
    #9
  10. Andy Lomax

    Andy Lomax Guest

    On 24 Jun 2005 07:25:06 -0700, "Shaun" <> wrote:

    >>Bad design.

    >
    >These are NOT badly designed methods. They are, in fact, recommended.
    >Rather than rant on why, I'll point you to Herb Sutter's "Exceptional
    >C++" series.


    Already got 6 C++ and STL books, with a 7th in the post, so I'm afraid
    I'll have to pass up on that. It'll take me to the next ice age just
    to read this lot.

    I don't suppose you could summarise? :)

    Cheers

    AL
     
    Andy Lomax, Jun 24, 2005
    #10
  11. Andy Lomax

    Shaun Guest

    "I don't suppose you could summarise?"

    Now that I think of it it may be Scott Meyers book, "Effective C++" (if
    not both books), however the guidline is generally stated as follows:

    "prefer pass-by-reference to pass-by-value"

    you can pair this with another oft-stated guidline, "use const whenever
    possible".

    As to WHY these guidlines are states as they are, I don't have my own
    copies nearby and so won't risk improperly parapharasing or putting
    words in the authors' mouths but you may find some good discussion on
    the matter elsewhere on the web. I provide tree links below to get you
    started.

    http://www.informit.com/articles/article.asp?p=373338&seqNum=2
    http://www.goingware.com/tips/parameters/parameters.html
    http://www.tantalon.com/pete/cppopt/asyougo.htm
     
    Shaun, Jun 24, 2005
    #11
  12. Andy Lomax

    John Carson Guest

    "Andy Lomax" <abuse@[127.0.0.1]> wrote in message
    news:
    > On Fri, 24 Jun 2005 22:10:22 +1000, "John Carson"
    > <> wrote:
    >
    >> If you only want one string for the whole class, then the obvious
    >> thing to do is make it a static member.

    >
    > Sorry, not making myself clear. The class may be instantiated
    > thousands of times, but there may be only 4 or 5 unique values for the
    > string; I can't tell before runtime.
    >
    > Ideally, I'd like to declare a smart pointer, but I can't see how to
    > tie that in with the prototypes that the library vendor has given me.
    > I can create a smart ptr for the string when I first create the
    > string, before passing it into the library as a reference. However,
    > when the library eventually calls 'setstr' with this reference, I
    > can't turn it back into the original smart ptr without some sort of
    > search.


    I don't know what the smartness of the pointer has to do with anything.

    If you think it is worth the trouble (which seems doubtful to me), it seems
    that what you need is an auxiliary structure (say, a set since you can
    insert without invalidating addresses). You make the set a static member of
    the class and each object of the class has a pointer to an entry in this
    set. Try the following:

    #include<string>
    #include<set>
    #include <iostream>
    using namespace std;

    class vendor_bar
    {};

    class foo : vendor_bar {
    string* pstr;
    static set<string> s;
    public:
    virtual void setstr(const string& str_)
    { pstr = &(*s.insert(str_).first); }
    virtual string& getstr(void) { return *pstr; }
    };

    set<string> foo::s;

    // Note that, rather helpfully, set::insert will only make
    // an actual insertion if the string isn't already there.
    // If it is there, it just returns an iterator to the existing element.

    int main()
    {
    const int number=8;
    foo f[number];
    f[0].setstr("First");
    f[1].setstr("First");
    f[2].setstr("Second");
    f[3].setstr("Second");
    f[4].setstr("Third");
    f[5].setstr("Fourth");
    f[6].setstr("Fifth");
    f[7].setstr("Fifth");

    for(int i=0; i<number; ++i)
    cout << "i: " << i << " string is: " << f.getstr() << endl;
    }

    You could further complicate this by refcounting the entries in the set and
    removing an entry when no object stored its address, but this seems unlikely
    to be worth it.

    --
    John Carson
     
    John Carson, Jun 24, 2005
    #12
  13. Andy Lomax

    Andy Lomax Guest

    Thanks - I'll check the references. I think your other reference was
    probably Meyer #20 - 'Prefer pass-by-reference-to-const to
    pass-by-value'; I found that this morning.

    Cheers

    AL
     
    Andy Lomax, Jun 27, 2005
    #13
  14. * Shaun:
    > "I don't suppose you could summarise?"
    >
    > Now that I think of it it may be Scott Meyers book, "Effective C++" (if
    > not both books), however the guidline is generally stated as follows:
    >
    > "prefer pass-by-reference to pass-by-value"
    >
    > you can pair this with another oft-stated guidline, "use const whenever
    > possible".


    You remember the first one incorrectly (unless Scott Meyers has written in
    his sleep); the second is advice I've given myself, publicly.

    Anyway it doesn't have anything to do with the interface Andy was given:

    void setstr(const std::string& str)
    std::string& getstr(void)

    It's badly designed, period.

    It also indicates that the person writing it is more familiar with C than
    C++.

    The only way it could make sense would be for 'getstr' to return a reference
    to a non-per-instance member, e.g. a static member or global.

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Jun 27, 2005
    #14
    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. Siemel Naran
    Replies:
    4
    Views:
    839
    Micah Cowan
    Jan 12, 2005
  2. Replies:
    1
    Views:
    423
    myork
    May 23, 2007
  3. Replies:
    1
    Views:
    410
    Victor Bazarov
    May 23, 2007
  4. Quek
    Replies:
    3
    Views:
    352
  5. blangela
    Replies:
    8
    Views:
    689
    Erik Wikström
    Sep 26, 2008
Loading...

Share This Page