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 skrev:
[color=blue]
> 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.[/color]

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

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

/Peter[color=blue]
>
> TIA
>
> string PublishedProductsRepo :: CreateStatement() const {
> string statement;
>
> statement ="SELECT DISTINCT "
> "* "
> "FROM "
> "map";
> return string(statement); }[/color]

* jimjim:[color=blue]
>
> 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); }[/color]

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?

In article <D_yVf.42416$wl.27960@text.news.blueyonder.co.uk >,
"jimjim" <netuser@blueyonder.co.uk> wrote:
[color=blue]
> 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); }[/color]

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. wrote:
[color=blue]
> 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.[/color]

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.
[color=blue]
> Or maybe he was intentionally trying to obfuscate the code?[/color]

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

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

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

On Sun, 26 Mar 2006 18:20:50 +0200, "Alf P. Steinbach"
<alfps@start.no> wrote:[color=blue]
>Whether the copy constructor is called depends on your compiler and the
>context of the CreateStatement call.[/color]

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

On Sun, 26 Mar 2006 18:27:14 GMT, "Tomás" <NULL@NULL.NULL> wrote:
[color=blue]
>3) A proficient programmer would probably write:
>
>string PublishedProductsRepo :: CreateStatement() const
>{
> return
> "SELECT DISTINCT "
> "* "
> "FROM "
> "map";
>}[/color]

That should be:

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

--
Bob Hairgrove
NoSpamPlease@Home.com

* Roland Pibinger:[color=blue]
> On Sun, 26 Mar 2006 18:20:50 +0200, "Alf P. Steinbach"
> <alfps@start.no> wrote:[color=green]
>> Whether the copy constructor is called depends on your compiler and the
>> context of the CreateStatement call.[/color]
>
> Whether the copy constructor is elided depends on your compiler, your
> used compliler switches and the context of the CreateStatement call.[/color]

Yes, I think you have understood that correctly.

[color=blue]
> In general, it's not recommendable to make your code dependent on
> language hacks like RVO.[/color]

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?

Tomás wrote:
[color=blue]
> 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");[/color]

The compiler is required to directly call the constructor, even if = is
used.
[color=blue]
> 2) There's no point in creating that nameless temporary in the "return"
> statement. It could simply be:
>
> return statement;[/color]

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(statemen t))))));
[color=blue]
> 3) A proficient programmer would probably write:
>
> string PublishedProductsRepo :: CreateStatement() const
> {
> return
> "SELECT DISTINCT "
> "* "
> "FROM "
> "map";
> }[/color]

And the next level of proficiency avoids this AntiPattern:

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

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!

* Bob Hairgrove:[color=blue]
> On Sun, 26 Mar 2006 18:27:14 GMT, "Tomás" <NULL@NULL.NULL> wrote:
>[color=green]
>> 3) A proficient programmer would probably write:
>>
>> string PublishedProductsRepo :: CreateStatement() const
>> {
>> return
>> "SELECT DISTINCT "
>> "* "
>> "FROM "
>> "map";
>> }[/color]
>
> That should be:
>
> std::string PublishedProductsRepo :: CreateStatement() const
> {
> return std::string(
> "SELECT DISTINCT "
> "* "
> "FROM "
> "map");
> }[/color]

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?

On Sun, 26 Mar 2006 22:44:05 +0200, "Alf P. Steinbach"
<alfps@start.no> wrote:[color=blue]
>* Roland Pibinger:[color=green]
>> In general, it's not recommendable to make your code dependent on
>> language hacks like RVO.[/color]
>
>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.[/color]

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

Best regards.
Roland Pibinger

Bob Hairgrove wrote:
[color=blue]
> That should be:
>
> std::string PublishedProductsRepo :: CreateStatement() const
> {
> return std::string(
> "SELECT DISTINCT "
> "* "
> "FROM "
> "map");
> }[/color]

Why?

--
Phlip
http://www.greencheese.org/ZeekLand <-- NOT a blog!!!

* Phlip:[color=blue]
> Tomás wrote:
>[color=green]
>> 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");[/color]
>
> The compiler is required to directly call the constructor, even if = is
> used.[/color]

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

[color=blue][color=green]
>> 2) There's no point in creating that nameless temporary in the "return"
>> statement. It could simply be:
>>
>> return statement;[/color]
>
> 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.[/color]

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.

[color=blue]
> 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(statemen t))))));[/color]

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?