Stroustrup 5.9 exercise 11

[arnuld]

it works fine without any trouble. i want to have advice on improving
the code from any angle like readability, maintenance etc:

---------- PROGRAMME ------------
/* Stroustrup, 5.9, exercise 11

STATEMENT:
Read a sequence of words from the input. use "quit" as the word
to terminate the input. Print the words in the order they were
entered. don't print a word twice.modify the programme to sort the
words before printing them.

*/

#include<iostre am>
#include<string >
#include<vector >

int main()
{
std::vector<std ::stringcollect _input;

std::string input_word;
std::cin >input_word;

for(int i=0; input_word != "quit"; ++i)
{
collect_input.p ush_back(input_ word);
std::cin >input_word;
}

std::cout << "\n *** Printing WOrds ***\n";

for(unsigned int i=0; i < collect_input.s ize(); ++i)
std::cout << collect_input[i]
<< '\n';

return 0;
}

----------- OUTPUT ----------------
[arch@voodo tc++pl]$ g++ -ansi -pedantic -Wall -Wextra -O
ex_5.9-11.cpp
[arch@voodo tc++pl]$ ./a.out
like this
quitting
quite
and morq quotwe FINISHED quiT quit

*** Printing WOrds ***
like
this
quitting
quite
and
morq
quotwe
FINISHED
quiT
[arch@voodo tc++pl]$

[arnuld]

SORRY, i just forgot to "sort" the words:

-------- PROGRAMME --------
/* Stroustrup, 5.9, exercise 11

STATEMENT:
Read a sequence of words from the input. use "quit" as the word
to terminate the input. Print the words in the order they were
entered. don't print a word twice.modify the programme to sort the
words before printing them.

*/

#include<iostre am>
#include<string >
#include<vector >
#include<algori thm>

int main()
{
std::vector<std ::stringcollect _input;

std::string input_word;
std::cin >input_word;

for(int i=0; input_word != "quit"; ++i)
{
collect_input.p ush_back(input_ word);
std::cin >input_word;
}

std::cout << "\n *** Printing Sorted Words ***\n";

// sorting the input
sort(collect_in put.begin(), collect_input.e nd());

for(unsigned int i=0; i < collect_input.s ize(); ++i)
std::cout << collect_input[i]
<< '\n';

return 0;
}

-------- OUTPUT ----------
[arch@voodo tc++pl]$ g++ -ansi -pedantic -Wall -Wextra -O
ex_5.9-11.cpp
[arch@voodo tc++pl]$ ./a.out
like this and wahout
Quit quiT and quit

*** Printing Sorted Words ***
Quit
and
and
like
quiT
this
wahout
[arch@voodo tc++pl]$

[arnuld]

On Apr 9, 2:07 pm, "arnuld" <geek.arn...@gm ail.comwrote:

i just did not get 2 things:

// sorting the input
sort(collect_in put.begin(), collect_input.e nd());

why "std::sort" does not work ?? (as "algorithm" is a standard
library)

-------- OUTPUT ----------
[arch@voodo tc++pl]$ g++ -ansi -pedantic -Wall -Wextra -O
ex_5.9-11.cpp
[arch@voodo tc++pl]$ ./a.out
like this and wahout
Quit quiT and quit

*** Printing Sorted Words ***
Quit
and
and
like
quiT
this
wahout
[arch@voodo tc++pl]$

why "Quit" came before "and" ?

[Zeppe]

arnuld wrote:

it works fine without any trouble. i want to have advice on improving
the code from any angle like readability, maintenance etc:

The program is very well written, I like your style. I have to be very
meticolous to find something that can be improved :)

#include<iostre am>
#include<string >
#include<vector >

It's just a matter of style, but almost anybody puts a space between
#include and the angular parenthesis, because it improves the
readability a lot.

int main()
{
std::vector<std ::stringcollect _input;

std::string input_word;
std::cin >input_word;

for(int i=0; input_word != "quit"; ++i)
{
collect_input.p ush_back(input_ word);
std::cin >input_word;
}

Here I have two little suggestions. The fist is to eliminate the
repetition of the input reading, the other is that a for cycle should
perform a test that is strictly related on the variable that is being
incremented. If you don't have to iterate in some way through a
sequence, and the test is a little bit particular, it's clearer to write
it without the for. I would prefer in this situation something like:

std::vector<std ::stringcollect _input;

while(true){
std::string input_word;
std::cin >input_word;
if(input_word == "quit")
break;
else
collect_input.p ush_back(input_ word);
}

In this program is pointless to perform such a change, but in bigger
programs is very important to understand very quickly and easily the
meaning of each piece of code.

Another note: if the performances are not a priority, it is better to
declare the variables as close as possible to the point in which they
are used. For example, the string "input_word " is not that important in
the whole program, and it's used just in the for. If I'm able to declare
it into the for, I reduce the visibility of the variable to the piece of
code in which I actually use it, and I reduce the chance of error
improving the readability.

std::cout << "\n *** Printing WOrds ***\n";

for(unsigned int i=0; i < collect_input.s ize(); ++i)
std::cout << collect_input[i]
<< '\n';

Use std::size_t instead of unsigned int when you iterate on a vector. In
the std::cout line, I'd have put all the code in one line, since there
is no readability issue in separating it. Also, pay attention with the
for without parenthesis, there is nothing bad with them, but it has to
be very obvious that there is only an instruction behind them.
Otherwise, they can generate errors quite hard to detect.
Regards,

Zeppe

[sonison.james]

On Apr 9, 2:10 pm, "arnuld" <geek.arn...@gm ail.comwrote:

On Apr 9, 2:07 pm, "arnuld" <geek.arn...@gm ail.comwrote:

i just did not get 2 things:

// sorting the input
sort(collect_in put.begin(), collect_input.e nd());

why "std::sort" does not work ?? (as "algorithm" is a standard
library)

-------- OUTPUT ----------
[arch@voodo tc++pl]$ g++ -ansi -pedantic -Wall -Wextra -O
ex_5.9-11.cpp
[arch@voodo tc++pl]$ ./a.out
like this and wahout
Quit quiT and quit

*** Printing Sorted Words ***
Quit
and
and
like
quiT
this
wahout
[arch@voodo tc++pl]$

why "Quit" came before "and" ?

Why not use "set", it is ordered and will remove duplicates too.

#include <iostream>
#include <set>
#include <string>
#include <algorithm>
#include <iterator>
using namespace std;

int main()
{
string inp;
set< string s;
cin >inp;
while( inp != "quit" )
{
s.insert( inp );
cin >inp;
}

copy( s.begin(), s.end(), ostream_iterato r< string >(cout, "\n" ) );

return 0;
}

[Zeppe]

arnuld wrote:

>On Apr 9, 2:07 pm, "arnuld" <geek.arn...@gm ail.comwrote:

i just did not get 2 things:

> // sorting the input
sort(collect_in put.begin(), collect_input.e nd());

why "std::sort" does not work ?? (as "algorithm" is a standard
library)

it does not?

>
why "Quit" came before "and" ?

because it starts with a capital letter.

Regards,

Zeppe

[Daniel T.]

"arnuld" <ge*********@gm ail.comwrote:

it works fine without any trouble. i want to have advice on improving
the code from any angle like readability, maintenance etc:

---------- PROGRAMME ------------
/* Stroustrup, 5.9, exercise 11

STATEMENT:
Read a sequence of words from the input. use "quit" as the word
to terminate the input. Print the words in the order they were
entered. don't print a word twice.modify the programme to sort the
words before printing them.

*/

#include<iostre am>
#include<string >
#include<vector >

int main()
{
std::vector<std ::stringcollect _input;

std::string input_word;
std::cin >input_word;

for(int i=0; input_word != "quit"; ++i)
{
collect_input.p ush_back(input_ word);
std::cin >input_word;
}

You create the variable 'i' and increment it, but never use it for
anything. Also, you never check for input failure. I suggest something
like this instead:

std::string input_word;
while ( std::cin >input_word && input_word != "quit" )
{
collect_input.p ush_back( input_word );
}

std::cout << "\n *** Printing WOrds ***\n";

for(unsigned int i=0; i < collect_input.s ize(); ++i)
std::cout << collect_input[i]
<< '\n';

A more idiomatic way of doing the above is to use std::copy:

std::copy( collect_input.b egin(), collect_input.e nd(),
ostream_iterato r<std::string> ( cout, "\n" ) );

return 0;
}

One last thing, I think you will find that the above program will print
words twice if they are entered twice. Look up std::set.

[arnuld]

On Apr 9, 4:38 pm, "Daniel T." <danie...@earth link.netwrote:

"arnuld" <geek.arn...@gm ail.comwrote:

You create the variable 'i' and increment it, but never use it for
anything. Also, you never check for input failure. I suggest something
like this instead:

std::string input_word;
while ( std::cin >input_word && input_word != "quit" )
{
collect_input.p ush_back( input_word );
}

that is good, i will use it. IIRC, i once read the same in K&R2:

while((c = getchar()) != EOF && i < MAXLENGTH)

just mentioned it as it came to my mind

std::cout << "\n *** Printing WOrds ***\n";

for(unsigned int i=0; i < collect_input.s ize(); ++i)
std::cout << collect_input[i]
<< '\n';

A more idiomatic way of doing the above is to use std::copy:

std::copy( collect_input.b egin(), collect_input.e nd(),
ostream_iterato r<std::string> ( cout, "\n" ) );

return 0;
}

2 questions:

1.) why do i need to "copy" the whole vector ?

2.) is "std::cout" is less maintainable/readable than
"ostream_iterat or" ?

One last thing, I think you will find that the above program will print
words twice if they are entered twice. Look up std::set.

i tried std::set, it does not make any sense to me, as of now. it is
on page 491, section 17.4.3 of Stroustrup (special edition).
BTW, this is the best i have come up with:

--------- PROGRAMME ------------
/* Stroustrup, 5.9, exercise 11

STATEMENT:
Read a sequence of words from the input. use "quit" as the word
to terminate the input. Print the words in the order they were
entered. don't print a word twice.modify the programme to sort the
words before printing them.

*/

#include<iostre am>
#include<string >
#include<algori thm>
#include<vector >
#include<set>
#include<iterat or>

int main()
{
std::string input_word;
std::vector<std ::stringcollect _input, removed_duplica tes;

while(std::cin >input_word && input_word != "quit")
{
collect_input.p ush_back(input_ word);
}
std::cout << "\n *** Printing Sorted Words ***\n";

// sorting the input
sort(collect_in put.begin(), collect_input.e nd());
unique_copy(col lect_input.begi n(), collect_input.e nd(),
back_inserter(r emoved_duplicat es));

for(std::size_t i=0; i < removed_duplica tes.size(); ++i)
{
std::cout << removed_duplica tes[i] << '\n';
}

return 0;
}

---------- OUTPUT -------------
[arch@voodo tc++pl]$ g++ -ansi -pedantic -Wall -Wextra -O
ex_5.9-11_modified.cpp
[arch@voodo tc++pl]$ ./a.out
like this
and this
and this
done quit

*** Printing Sorted Words ***
and
done
like
this
[arch@voodo tc++pl]$ ./a.out

[Anand Hariharan]

On Apr 9, 5:34 am, Zeppe <[e-mail] rote:

>
Another note: if the performances are not a priority, it is better to
declare the variables as close as possible to the point in which they
are used.

What does "declaring variables close to first use" have to do with
performance?

- Anand

[Daniel T.]

"arnuld" <ge*********@gm ail.comwrote:

On Apr 9, 4:38 pm, "Daniel T." <danie...@earth link.netwrote:
"arnuld" <geek.arn...@gm ail.comwrote:

std::cout << "\n *** Printing WOrds ***\n";

for(unsigned int i=0; i < collect_input.s ize(); ++i)
std::cout << collect_input[i]
<< '\n';

A more idiomatic way of doing the above is to use std::copy:

std::copy( collect_input.b egin(), collect_input.e nd(),
ostream_iterato r<std::string> ( cout, "\n" ) );

return 0;
}

2 questions:

1.) why do i need to "copy" the whole vector ?

You have to copy the whole vector to cout. That is what you are doing
when you print all the strings.

2.) is "std::cout" is less maintainable/readable than
"ostream_iterat or" ?

No, but the loop itself is.

for ( std::vector<std ::string>::iter ator i = collected_input .begin();
i != collected_input .end(); ++i )
{
cout << *i << '\n';
}

The above works, but if you decide to change the container to anything
other than a vector, you have to change the loop also.

for ( unsigned i = 0; i < collected_input .size(); ++i )
{
cout << collected_input[i] << '\n';
}

The above also works but only for vectors and deques, not for lists or
sets.

One last thing, I think you will find that the above program will print
words twice if they are entered twice. Look up std::set.

i tried std::set, it does not make any sense to me, as of now. it is
on page 491, section 17.4.3 of Stroustrup (special edition).

With std::set, you insert the item instead of using push_back and it
keeps the items sorted.

BTW, this is the best i have come up with:

What you came up with is great for satisfying the last sentence of the
requirement, but not the first 3.

The hardest part of this exorcise is to remove duplicates *without*
sorting the input.