C++ Primer ex 3.14

[arnuld]

i have solved it. any suggestions for improving the code:

/* C++ Primer - 4/e
* chapter 3

* exercise 3.14

* STATEMENT
read some text into a vector,storing each word as an elelment in
the vector. transform each word into the uppercase letter. print the
transofrmed elelments from the vector as eight words per line.

*/

#include <iostream>
#include <vector>
#include <string>

int main()
{
std::cout << "please enter some words seperated by newlines" <<
std::endl;

std::string a_word;
std::vector<std::string> svec;
while(std::cin >> a_word)
{
svec.push_back(a_word);
}

for(std::vector<std::string>::iterator iter=svec.begin(); iter !=
svec.end(); ++iter)
{
for(std::string::size_type ix=0; ix != (*iter).size(); ++ix)
{
((*iter)[ix]) = toupper((*iter)[ix]);

/* i call it the MESSY solution.
isn't there anything better ? */
}
}

int counter = 0;
for(std::vector<std::string>::const_iterator iter=svec.begin(); iter !=
svec.end(); ++iter)
{
if(counter == 7)
{
std::cout << *iter << std::endl;
counter = 0;
}
else
{
std::cout << *iter << " ";
++counter;
}
}

std::cout << std::endl;

return 0;
}


======= OUTPUT ===========
[arnuld@arch cpp ]% g++ -ansi -pedantic -Wall -Wextra ex_03-14.cpp
[arnuld@arch cpp ]% ./a.out
please enter some words seperated by newlines
aRnUld
Fraser
was fallING
out OFF his TraCKL from some MONTHS but NOW hE iS bAcK

ARNULD FRASER WAS FALLING OUT OFF HIS TRACKL
FROM SOME MONTHS BUT NOW HE IS BACK

[arnuld@arch cpp ]%

 

[Victor Bazarov]

[..]
for(std::vector<std::string>::iterator iter=svec.begin(); iter !=
svec.end(); ++iter)
{
for(std::string::size_type ix=0; ix != (*iter).size(); ++ix)
{
((*iter)[ix]) = toupper((*iter)[ix]);

/* i call it the MESSY solution.
isn't there anything better ? */

You mean, like writing a functor which will call 'transform' and
then calling 'for_each' here?

}
}
[..]

V

 

[arnuld]

[..]
for(std::vector<std::string>::iterator iter=svec.begin(); iter !=
svec.end(); ++iter)
{
for(std::string::size_type ix=0; ix != (*iter).size(); ++ix)
{
((*iter)[ix]) = toupper((*iter)[ix]);

/* i call it the MESSY solution.
isn't there anything better ? */

You mean, like writing a functor which will call 'transform' and
then calling 'for_each' here?

i am still chapter 3 of C++ Primer 4/e yet but i have read soem parts of
Stroustrup some months ago, so IIRC, "for_each" is a Standard Library
Algorithm.

BTW, i will stop here because i think the author have introduced some
basic features only, so within those "basic features", withing those
constraints, my programme is ok. thanks Victor for reminding me that

 

[James Kanze]

i have solved it.

No you haven't, but you've run into a very subtle issue.

any suggestions for improving the code:

/* C++ Primer - 4/e
* chapter 3

* exercise 3.14

* STATEMENT
read some text into a vector,storing each word as an elelment in
the vector. transform each word into the uppercase letter. print the
transofrmed elelments from the vector as eight words per line.
*/

#include <iostream>
#include <vector>
#include <string>

You're missing at least one include.

int main()
{
std::cout << "please enter some words seperated by newlines" <<
std::endl;

std::string a_word;
std::vector<std::string> svec;
while(std::cin >> a_word)
{
svec.push_back(a_word);
}

for(std::vector<std::string>::iterator iter=svec.begin(); iter !=
svec.end(); ++iter)
{
for(std::string::size_type ix=0; ix != (*iter).size(); ++ix)
{
((*iter)[ix]) = toupper((*iter)[ix]);
/* i call it the MESSY solution.
isn't there anything better ? */

I call it a wrong solution. On my system, if the user enters an
accented character, it passes a negative value to "toupper".
For the one argument version of toupper (declared in <ctype.h>),
"the argument is an int, the value of which shall be
representable as an unsigned char or shall equal the value of
the macro EOF." This is a pre-condition; violating it results
in undefined behavior. At the very least, you need to write:

(*iter)[ ix ] = toupper( static_cast< unsigned char >( (*iter)
[ ix ] ) ;

(In practice, of course, I'd recommend using string iterators
for the inner loop as well. It's more C++'ish.)

As you can see by the name of the header, the single argument
version of toupper is in fact a C function (included by
reference in C++). The C++ variants are in <locale> (and have
been designed to be particularly complicated to use). In
C++, there is a convenience function, std::toupper, but most of
the time, you'll probably want to obtain a ctype facet, and work
with that:

std::ctype< char > const& ctype
= std::use_facet< std::ctype< char > >( std::locale() ) ;

There's a ctype::toupper which works on strings, but only on
char[] strings. (Don't ask me how they justify that.) So once
again, you're stuck with a loop. Or you embed the call in a
functional object, and use std::transform; this occurs so often
in my own code that I have such functional objects in my
toolbox, and would simply write:

std::transform( iter->begin(), iter->end(),
iter->begin(),
CTypeToUpper() ) ;

Writing a robust version of such a functional object is trickier
than it looks, because you have to be concerned with the
lifetime of the facet (e.g. if the locale is a temporary, or if
someone changes it during the lifetime of the functional
object); something sufficient for a program like yours (and most
programs, in fact), however, shouldn't be that difficult, and
would serve as an interesting introduction to <locale>.
(Implementing one which uses the <ctype.h> one argument form of
toupper is even easier. Just don't forget the coversion to
unsigned char.)

 

[arnuld]

I call it a wrong solution. On my system, if the user enters an
accented character, it passes a negative value to "toupper".
For the one argument version of toupper (declared in <ctype.h>),
"the argument is an int, the value of which shall be
representable as an unsigned char or shall equal the value of
the macro EOF." This is a pre-condition; violating it results
in undefined behavior. At the very least, you need to write:

(*iter)[ ix ] = toupper( static_cast< unsigned char >( (*iter)
[ ix ] ) ;

(In practice, of course, I'd recommend using string iterators
for the inner loop as well. It's more C++'ish.)

i tried that but it does not work, i get a compile-time error:

for(std::string::iterator siter=(*iter).begin();
siter !=(*iter).end(); ++siter)
{
((*iter)(*siter)) = toupper(static_cast<unsigned char>((*iter)(*siter)));
}



{arnuld@arch cpp }% g++ -ansi -pedantic -ggdb -Wall -Wextra test.cpp
test.cpp: In function ‘int main()’:
test.cpp:32: error: no match for call to ‘(std::basic_string<char, std::char_traits<char>, std::allocator<char> >) (char&)’
test.cpp:32: error: no match for call to ‘(std::basic_string<char, std::char_traits<char>, std::allocator<char> >) (char&)’
{arnuld@arch cpp }%

 

[BobR]

i tried that but it does not work, i get a compile-time error:

for(std::string::iterator siter=(*iter).begin();
siter !=(*iter).end(); ++siter){
((*iter)(*siter)) = toupper(static_cast<unsigned

char>((*iter)(*siter)));

Did you try:

}

Forget the iterators for a minute. Let's look at straight indexing.

std::vector<std::string> svec;
// fill with strings

for( std::size_t iter(0); iter < svec.size(); ++iter){
for( std::size_t ix(0); ix < svec.at(iter).size(); ++ix){
svec.at(iter).at(ix) // this is char ix of string in svec iter

// What you did above with your iterators is:
// svec.at(iter).at(iter).at(ix)


// int main()
{
std::vector<std::string> svec;
svec.push_back("Skunk");
svec.push_back("Snail");
svec.push_back("Slug");
for(std::vector<std::string>::iterator iter( svec.begin() );
iter != svec.end(); ++iter){
for( std::string::iterator siter( (*iter).begin() );
siter != (*iter).end(); ++siter){
// > ((*iter)(*siter)) = toupper(static_cast<unsigned
char>((*iter)(*siter)));

(*siter) = std::toupper(static_cast<unsigned
char>( (*siter) ) );

} // for(string)
} // for(svec)

std::copy( svec.begin(), svec.end(),
std:stream_iterator<std::string>( std::cout, "\n") );
cout<<std::endl;
// return 0;
}
/* - output -
SKUNK
SNAIL
SLUG
*/

 

[arnuld]

Did you try:
(*siter) = toupper(static_cast<unsigned char>(*siter) );

yes, it works i got the logic of it, as (*iter) will remain
pointing to one place till we finish through the test conditon of
inner for loop.

Forget the iterators for a minute. Let's look at straight indexing.

std::vector<std::string> svec;
// fill with strings

for( std::size_t iter(0); iter < svec.size(); ++iter){
for( std::size_t ix(0); ix < svec.at(iter).size(); ++ix){
svec.at(iter).at(ix) // this is char ix of string in svec iter

// What you did above with your iterators is:
// svec.at(iter).at(iter).at(ix)

i know that and did write this code earlier, my code was different in
the initialisation phase. i used:

std::vector<int>::size_type ix= svec.begin()

subscriptiing is uch easier but as authors of C++ Primer put it
subscripting is not vailable for every "Class Template" or
Std. Lib. Container but iterators are available for every
container/Class-Template, since, most of the time, i will be writing
C++ code using the Standard Library (e.g. Vectors & Strings rather than
Arrays and Pointers) i try to learn more about the use and behaviour
of iterators. this exercise was my one attempt.

// int main()
{
std::vector<std::string> svec;
svec.push_back("Skunk");
svec.push_back("Snail");
svec.push_back("Slug");
for(std::vector<std::string>::iterator iter( svec.begin() );
iter != svec.end(); ++iter){
for( std::string::iterator siter( (*iter).begin() );
siter != (*iter).end(); ++siter){
// > ((*iter)(*siter)) = toupper(static_cast<unsigned
char>((*iter)(*siter)));

(*siter) = std::toupper(static_cast<unsigned
char>( (*siter) ) );

} // for(string)
} // for(svec)

yes, this is what you changed and it runs

std::copy( svec.begin(), svec.end(),
std:stream_iterator<std::string>( std::cout, "\n") );
cout<<std::endl;
// return 0;
}

i have included "algorithm" header but this piece of code does not
run.i get an error:

{arnuld@arch cpp }% g++ -ansi -pedantic -Wall -Wextra test.cpp
test.cpp: In function ‘int main()’:
test.cpp:43: error: ‘ostream_iterator’ is not a member of ‘std’
test.cpp:43: error: expected primary-expression before ‘>’ token
test.cpp:43: warning: left-hand operand of comma has no effect


i even tried to remove the "std::" scope operator but then it says,
"ostream_iterator was not declared in this scope".

 

[BobR]

yes, it works i got the logic of it, as (*iter) will remain
pointing to one place till we finish through the test conditon of
inner for loop.


i know that and did write this code earlier, my code was different in
the initialisation phase. i used:

std::vector<int>::size_type ix= svec.begin()

Did that work? It shouldn't. '.begin()' returns an iterator, not a size_type
(usually an 'size_t').
BTW,
sometype mine( 0 );
and
sometype mine = 0;
do the same thing in the end. I prefer the first, while many prefer the
second(they like to *see* the assignment (=)).

subscriptiing is uch easier but as authors of C++ Primer put it
subscripting is not vailable for every "Class Template" or
Std. Lib. Container but iterators are available for every
container/Class-Template, since, most of the time, i will be writing
C++ code using the Standard Library (e.g. Vectors & Strings rather than
Arrays and Pointers) i try to learn more about the use and behaviour
of iterators. this exercise was my one attempt.

I was just using 'indexing' to illustrate your mistake. Definately learn
both.

yes, this is what you changed and it runs


i have included "algorithm" header but this piece of code does not
run.i get an error:

{arnuld@arch cpp }% g++ -ansi -pedantic -Wall -Wextra test.cpp
test.cpp: In function â?~int main()â?T:
test.cpp:43: error: â?~ostream_iteratorâ?T is not a member of â?~stdâ?T
test.cpp:43: error: expected primary-expression before â?~>â?T token
test.cpp:43: warning: left-hand operand of comma has no effect

i even tried to remove the "std::" scope operator but then it says,
"ostream_iterator was not declared in this scope".

My 'TestBench' probably pulls in every header in the system, either through
the many 'tests' in it(I need to clean it up), or through 'wxWidgets'. I'm
sorry for not noticing a missed one.
'std::copy' should be in <algorithm>.
You may need <iterator> besides <iostream>(, <ostream>) for the
'ostream::iterator'.

Try that. Re-post if you still have trouble, and I'll track it down.

 

[James Kanze]

I call it a wrong solution. On my system, if the user enters an
accented character, it passes a negative value to "toupper".
For the one argument version of toupper (declared in <ctype.h>),
"the argument is an int, the value of which shall be
representable as an unsigned char or shall equal the value of
the macro EOF." This is a pre-condition; violating it results
in undefined behavior. At the very least, you need to write:
(*iter)[ ix ] = toupper( static_cast< unsigned char >( (*iter)
[ ix ] ) ;
(In practice, of course, I'd recommend using string iterators
for the inner loop as well. It's more C++'ish.)

i tried that but it does not work, i get a compile-time error:

for(std::string::iterator siter=(*iter).begin();
siter !=(*iter).end(); ++siter)

Using iter->begin() and iter->end() instead of (*iter).begin(),
(*iter).end() is perhaps more idiomatic.

{
((*iter)(*siter)) = toupper(static_cast<unsigned char>((*iter)(*siter)));

What on earth is "(*iter)(*siter)" supposed to mean? With
siter, you're iterating through a string, so just:

*siter = toupper( static_cast< unsigned char >( *siter ) ;

should be all you need. And whatever you need, you can't just
juxtapose two iterator dereferences---regardless of the types,
there is no place in C++ where you can simply juxtapose two
value expressions without an operator.

}

{arnuld@arch cpp }% g++ -ansi -pedantic -ggdb -Wall -Wextra test.cpp
test.cpp: In function ?int main()?:
test.cpp:32: error: no match for call to ?(std::basic_string<char, std::char_traits<char>, std::allocator<char> >) (char&)?
test.cpp:32: error: no match for call to ?(std::basic_string<char, std::char_traits<char>, std::allocator<char> >) (char&)?
{arnuld@arch cpp }%

Apparently, the compiler is interpreting (*iter)(...) as a
function call (which it could be, if *iter returned a pointer to
a function or a class which implemented operator()()), and
complaining because you're trying to call a function on
something which doesn't support the operator()().