Well, now... My first suggestion was to merely remove the static.
Agree with this. "static" is a bug.
My second version of the function was written in a (presumed) context of an
existing one that works with a char* (not a const char*), as it is +/- usual
to do so in C).
But this is a rather strong limitation on client and my point is that
it leads leads poorer caller-side code.
C++ group so we should use C++ idioms rather than C but note that even
libc toupper return the value rather than modify its input.
While one could argue the speed advantage of avoiding a copy, I didn't.
You state that there are "no great benefits" to returning a string.
My statement is that there are even less great benefits to modifying in
place apart from a possible speed advantage. What other advantages do
you see to the in-place style?
I listed several benefits of returning it instead of modifying in
place. I'll agree that none of then are "great" "trumping arguments"
that justify 100% of returning vs 0% of in-place but I certainly
believe that these advantages are sufficient unless you have good
reason to believe that the possible speed advantage will be needed
(probably should be able to measure it).
Code in the function is actually shorter than anything else,
so the complexity is on the caller side.
Generally, it is better to place complexity in the callee side than in
the caller side. Reason: code reuse. The callee function should be
written once and called multiple times so you pay the complexity only
once. If you impose complexity to the caller, you have to pay it
every times the function is called.
But there, the only complexity is e.g. going from
f(strToUpper(s))
to
strToUpper(s);
foo(s);
**If** modification is made in-place, I personally would prefer making a
side effect explicit by exactly preventing function call chaining.
Of course the chaining style strUnquote(strToUpper(s)) is meant to
return by value and does not modify its argument. You'd just end up
modifying a temporary which is incorrect.
I agree with you that it is always desireable to make incorrect
constructs impossible (e.g. won't compile)
But how do you intend to *prevent* function chaining?
You may yourself only write in-place style functions but you will not
work with what are very common C++ idioms:
void strToUpperInPlace( std::string & s);
There is nothing in the above signature that prevent function chaining:
std::string s = "BlahBlahBlah";
strToUpperInPlace(s); // works as intended
strToUpperInPlace(s.substr(0,5)); // Ooops compile but doesn't work
std::vector<std::string> vs;
// populate vector
strToUpperInPlace(vs[2]); // Oops compile but doesn't work.
Taking a pointer input void strToUpperInPlace( std::string * s)
makes it a bit more awkward to chain so would discourage function
chaining but not prevent it either.
As for const-correctness: it's merely a case of explicitly making a copy
if needed. Not exactly a deal-breaker. ;-)
Agree, none of the benefits are 100% deal breakers. But they are
small benefits that together lead to better code.
In my experience, with using the "in place" style, the programmers
will tend to end up creating a system where all the data are passed
are by non-const references and/or non-const pointers and when trying
to read/review/maintain such code base, you can never know if a
function argument is going to be modified or not sometimes deep down
the calling chain.
Code may even tend to pessimise itself cause you can never trust that
data you pass as input to a function will not be trampled somewhere
down the line so you may end up making a temporary copy "just in case"
for safety.
Yannick
