Re: What is the output of this program?
jbannon wrote:
dnguyenk@yahoo.com wrote:
//...
string ToUpper(const string& s1)
{
string s2;
for (int i = 0; i < s1.size(); ++i)
if (isalpha(s1[i])) s2[i] = toupper(s1[i]);
return s2;
}
int main()
{
string v1[5] = {"This", "is", "a", "small", "quizz"};
string v2[5];
for (int i = 0; i < 5; ++i) v2[i] = ToUpper(v1[i]);
//..Print out v2. What is the output?
return 0;
}
A mistake made me stare at the screen for a while ...
Could you give some hints to avoid similar mistakes systematically?
Thanks.
Ng.
<SNIP>
There are at least two problems in the code:
Actually, there's only one major problem: the code is not
idiomatic C++.
1. The statement if (isalpha(s1[i])) s2[i] = toupper(s1[i]); does not
append any non-alphabetic characters to s2. This was the easy bit.
The statement in question doesn't append anything to s2, ever.
That should be an easy bit as well. For the above bit of code
to make sense, he'd have to have initialized s2 with s1. While
the code could then be made to work, I don't think it's really
idiomatic C++. If you had a toupper functional object, the
idiomatic solution would be simply:
std::transform( s1.begin(), s1.end(),
std::back_inserter( s2 ),
toupper() ) ;
Regretfully, the standard doesn't define such an object. (One
could easily argue that this is a design flaw in <locale>.)
IMHO, if you consider this functionality usable, then it is
frequent enough to receive a name, and to get its own functional
object. (That's a big if, however, since there is no way to
make it work with e.g. UTF-8.)
2. This one was a bit harder and it took me a while to see it
after a bit of testing (I am a beginner).
There's no harm in being a beginner, and not knowing something,
but there's one thing that is bothering me: what books are you
using to learn C++, if they don't teach the basics of using a
container. Normally, I would expect a beginner to know how to
use an iterator, and even back_inserter, before he ever
encountered [].
String s2 is initialised to the empty string in the definition
string s2; by std::string's default contructor, thus the
expression s2[i] is invalid for any i > 0.
For any i. In the expression s[i], i, converted to a size_t (an
unsigned integral type) must be strictly less than s.size().
If you do as I did and replace operator[] with the
range-checked version s2.at(i) you will get an index out of
range exception being thrown. What was needed to fix it is a
line like
s2 += islower(s1[i]) ? toupper(s1[i]) : s1[i];
Given that all of the standard versions of toupper are defined
to return the argument if islower is false, the test is
unnecessary. Given that the one argument version has undefined
behavior when called with a char argument, however, you're still
in trouble.
which appends each converted character to s2. The trouble with
this of course is that there are, at least potentially, O(n)
allocations + 1 copy (the return statement). I think we can
get around this by calling s2.reserve(s1.size()) so that we
have only 1 allocation.
You could. Or you could simple suppose that the implementation
wasn't completely stupid, so you don't get an allocation for
every character, and that the number of allocations you do get
won't have an important effect on your program. If it later
turns out that you were wrong, it's no hassle to add the reserve
later. Until then, it's one unnecessary line which has to be
maintained.
I'm still wondering why main() uses arrays rather than vectors.
Because main() was around long before std::vector was invented.
--
James Kanze kanze.james@neuf.fr
Conseils en informatique orient?e objet/
Beratung in objektorientierter Datenverarbeitung
9 place S?mard, 78210 St.-Cyr-l'?cole, France +33 (0)1 30 23 00 34
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]