Weird return statement

Discussion in 'C++' started by jimjim, Mar 26, 2006.

  1. jimjim

    jimjim Guest

    Hello all,

    I ve come across the following code fragment and I was wondering why is the
    copy ctr called on return (rather than just returning the string statement
    obj.

    TIA

    string PublishedProductsRepo :: CreateStatement() const {
    string statement;

    statement ="SELECT DISTINCT "
    "* "
    "FROM "
    "map";
    return string(statement); }
    jimjim, Mar 26, 2006
    #1
    1. Advertising

  2. jimjim

    peter koch Guest

    jimjim skrev:

    > Hello all,
    >
    > I ve come across the following code fragment and I was wondering why is the
    > copy ctr called on return (rather than just returning the string statement
    > obj.


    The code below does not make much sense. I would have written it
    simply:

    string PublishedProductsRepo :: CreateStatement() const {
    return "SELECT DISTINCT "
    "* "
    "FROM "
    "map";
    }

    /Peter
    >
    > TIA
    >
    > string PublishedProductsRepo :: CreateStatement() const {
    > string statement;
    >
    > statement ="SELECT DISTINCT "
    > "* "
    > "FROM "
    > "map";
    > return string(statement); }
    peter koch, Mar 26, 2006
    #2
    1. Advertising

  3. * jimjim:
    >
    > I ve come across the following code fragment and I was wondering why is the
    > copy ctr called on return (rather than just returning the string statement
    > obj.
    >
    > string PublishedProductsRepo :: CreateStatement() const {
    > string statement;
    >
    > statement ="SELECT DISTINCT "
    > "* "
    > "FROM "
    > "map";
    > return string(statement); }


    Whether the copy constructor is called depends on your compiler and the
    context of the CreateStatement call.

    There is much that is unnecessary in the code, but it doesn't affect
    copy constructor calls. What's important re copy constructor calls is
    your compiler's optimizations.

    A straightforward coding of this function is

    std::string PublishedProductsRepo::CreateStatement() const
    {
    return "SELECT DISTINCT * FROM map";
    }

    Note the qualification with "std::".

    If this inline function definition is in a header file, which is likely,
    the lack of such qualification in the original code indicates there is a
    "using namespace std;" or "using std::string;" in the header file, which
    in turn indicates an incompetent programmer. In other words, if those
    reasonable assumptions hold, then this is not code to learn from.
    Rather, it's then code from which you can learn how to /not/ do things.

    For example:

    * Don't ever place "using namespace std;" in a header file.

    * Don't be clever when there is no need.

    * Preferentially don't name a function "ComputeCosine" when "cos" will
    do: name value-producing functions for what they produce, not how.

    --
    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, Mar 26, 2006
    #3
  4. jimjim

    Daniel T. Guest

    In article <D_yVf.42416$>,
    "jimjim" <> wrote:

    > I ve come across the following code fragment and I was wondering why is the
    > copy ctr called on return (rather than just returning the string statement
    > obj.
    >
    > TIA
    >
    > string PublishedProductsRepo :: CreateStatement() const {
    > string statement;
    >
    > statement ="SELECT DISTINCT "
    > "* "
    > "FROM "
    > "map";
    > return string(statement); }


    The person who wrote the code was probably worried because 'statement'
    is a temporary and he wasn't sure if he was returning a temporary or
    not. IE he didn't know the language very well.

    Or maybe he was intentionally trying to obfuscate the code?


    --
    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.
    Daniel T., Mar 26, 2006
    #4
  5. jimjim

    Phlip Guest

    Daniel T. wrote:

    > The person who wrote the code was probably worried because 'statement'
    > is a temporary and he wasn't sure if he was returning a temporary or
    > not. IE he didn't know the language very well.


    You meant: ...probably worried because 'statement' is a local and he wasn't
    sure if he was returning a reference to a local. So he returned a copy of a
    temporary instead.

    > Or maybe he was intentionally trying to obfuscate the code?


    The OP is advised to learn better C++ than that example before further
    improvements. This will probably be tricky, because if the code has many
    more examples like this then some of them might rely on undefined behavior.

    --
    Phlip
    http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
    Phlip, Mar 26, 2006
    #5
  6. jimjim

    Tomás Guest


    > string PublishedProductsRepo :: CreateStatement() const {
    > string statement;
    >
    > statement ="SELECT DISTINCT "
    > "* "
    > "FROM "
    > "map";
    > return string(statement); }


    Looks like the coder isn't very proficient. Here's a few bad things:

    1) The "statement" object is default constructed, and the an assignment
    is performed. This is inefficent -- it should have just been a
    construction:

    string statement("bla bla");


    2) There's no point in creating that nameless temporary in the "return"
    statement. It could simply be:

    return statement;


    3) A proficient programmer would probably write:

    string PublishedProductsRepo :: CreateStatement() const
    {
    return
    "SELECT DISTINCT "
    "* "
    "FROM "
    "map";
    }


    -Tomás
    Tomás, Mar 26, 2006
    #6
  7. On Sun, 26 Mar 2006 18:20:50 +0200, "Alf P. Steinbach"
    <> wrote:
    >Whether the copy constructor is called depends on your compiler and the
    >context of the CreateStatement call.


    Whether the copy constructor is elided depends on your compiler, your
    used compliler switches and the context of the CreateStatement call.
    In general, it's not recommendable to make your code dependent on
    language hacks like RVO.

    Best regards,
    Roland Pibinger
    Roland Pibinger, Mar 26, 2006
    #7
  8. On Sun, 26 Mar 2006 18:27:14 GMT, "Tomás" <> wrote:

    >3) A proficient programmer would probably write:
    >
    >string PublishedProductsRepo :: CreateStatement() const
    >{
    > return
    > "SELECT DISTINCT "
    > "* "
    > "FROM "
    > "map";
    >}


    That should be:

    std::string PublishedProductsRepo :: CreateStatement() const
    {
    return std::string(
    "SELECT DISTINCT "
    "* "
    "FROM "
    "map");
    }

    --
    Bob Hairgrove
    Bob Hairgrove, Mar 26, 2006
    #8
  9. * Roland Pibinger:
    > On Sun, 26 Mar 2006 18:20:50 +0200, "Alf P. Steinbach"
    > <> wrote:
    >> Whether the copy constructor is called depends on your compiler and the
    >> context of the CreateStatement call.

    >
    > Whether the copy constructor is elided depends on your compiler, your
    > used compliler switches and the context of the CreateStatement call.


    Yes, I think you have understood that correctly.


    > In general, it's not recommendable to make your code dependent on
    > language hacks like RVO.


    But not this.

    On the contrary, it's absolutely not a good idea to resort to premature
    optimization (Google for "premature optimization"): it wastes programmer
    time and may end up making your program less efficient rather than more.

    --
    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, Mar 26, 2006
    #9
  10. jimjim

    Phlip Guest

    Tomás wrote:

    > 1) The "statement" object is default constructed, and the an assignment
    > is performed. This is inefficent -- it should have just been a
    > construction:
    >
    > string statement("bla bla");


    The compiler is required to directly call the constructor, even if = is
    used.

    > 2) There's no point in creating that nameless temporary in the "return"
    > statement. It could simply be:
    >
    > return statement;


    I do know that Return Value Optimization could make one of the strings
    inside the function become an alias for the final string outside the
    function. But I don't know how aggressive that rule is. Suppose the copy
    constructor for std::string had a side effect that you could count. RVO will
    make one of those side effects go away. (That's why it's a permitted
    optimization and defined as "lossy"; so programmers will know better than to
    depend on the number of side-effects from such constructors.)

    Will RVO make all of this go away?

    return string(string(string(string(string(string(statement))))));

    > 3) A proficient programmer would probably write:
    >
    > string PublishedProductsRepo :: CreateStatement() const
    > {
    > return
    > "SELECT DISTINCT "
    > "* "
    > "FROM "
    > "map";
    > }


    And the next level of proficiency avoids this AntiPattern:

    http://c2.com/cgi/wiki?PerniciousIngrownSql

    --
    Phlip
    http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
    Phlip, Mar 26, 2006
    #10
  11. * Bob Hairgrove:
    > On Sun, 26 Mar 2006 18:27:14 GMT, "Tomás" <> wrote:
    >
    >> 3) A proficient programmer would probably write:
    >>
    >> string PublishedProductsRepo :: CreateStatement() const
    >> {
    >> return
    >> "SELECT DISTINCT "
    >> "* "
    >> "FROM "
    >> "map";
    >> }

    >
    > That should be:
    >
    > std::string PublishedProductsRepo :: CreateStatement() const
    > {
    > return std::string(
    > "SELECT DISTINCT "
    > "* "
    > "FROM "
    > "map");
    > }


    No, you don't need the explicit constructor call, because that
    constructor is not 'explicit'.

    For e.g. a std::auto_ptr you would need the explicit construction.

    For std::string you don't.

    --
    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, Mar 26, 2006
    #11
  12. On Sun, 26 Mar 2006 22:44:05 +0200, "Alf P. Steinbach"
    <> wrote:
    >* Roland Pibinger:
    >> In general, it's not recommendable to make your code dependent on
    >> language hacks like RVO.

    >
    >But not this.
    >
    >On the contrary, it's absolutely not a good idea to resort to premature
    >optimization (Google for "premature optimization"): it wastes programmer
    >time and may end up making your program less efficient rather than more.


    In this case RVO is the immature optimization :)
    I don't recommend any optimization at all.

    Best regards.
    Roland Pibinger
    Roland Pibinger, Mar 26, 2006
    #12
  13. jimjim

    Phlip Guest

    Bob Hairgrove wrote:

    > That should be:
    >
    > std::string PublishedProductsRepo :: CreateStatement() const
    > {
    > return std::string(
    > "SELECT DISTINCT "
    > "* "
    > "FROM "
    > "map");
    > }


    Why?

    --
    Phlip
    http://www.greencheese.org/ZeekLand <-- NOT a blog!!!
    Phlip, Mar 26, 2006
    #13
  14. * Phlip:
    > Tomás wrote:
    >
    >> 1) The "statement" object is default constructed, and the an assignment
    >> is performed. This is inefficent -- it should have just been a
    >> construction:
    >>
    >> string statement("bla bla");

    >
    > The compiler is required to directly call the constructor, even if = is
    > used.


    Tomás was referring to the assignment operator call, I think.


    >> 2) There's no point in creating that nameless temporary in the "return"
    >> statement. It could simply be:
    >>
    >> return statement;

    >
    > I do know that Return Value Optimization could make one of the strings
    > inside the function become an alias for the final string outside the
    > function. But I don't know how aggressive that rule is.


    Not very, it applies to temporaries and returning a named local
    variable. But that's enough here, and what kicks in here anyway is the
    "as if" rule. std::string is a class known by the compiler, and it can
    optimize as it wants to, regardless of the RVO rule.


    > Suppose the copy
    > constructor for std::string had a side effect that you could count. RVO will
    > make one of those side effects go away. (That's why it's a permitted
    > optimization and defined as "lossy"; so programmers will know better than to
    > depend on the number of side-effects from such constructors.)
    >
    > Will RVO make all of this go away?
    >
    > return string(string(string(string(string(string(statement))))));


    RVO /can/ make all of that go away, "whenever a temporary class object
    is copied using a copy constructor".

    Whether it will or not depends on the compiler etc.


    --
    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, Mar 26, 2006
    #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. Seong-Kook Shin
    Replies:
    1
    Views:
    488
    Richard Bos
    Jun 18, 2004
  2. Greenhorn
    Replies:
    15
    Views:
    812
    Keith Thompson
    Mar 6, 2005
  3. dorayme
    Replies:
    1
    Views:
    613
    richard
    Jan 21, 2011
  4. richard
    Replies:
    0
    Views:
    578
    richard
    Jan 21, 2011
  5. richard
    Replies:
    0
    Views:
    613
    richard
    Jan 21, 2011
Loading...

Share This Page