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<iostream>
#include<string>
#include<vector>

int main()
{
std::vector<std::string> collect_input;

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

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

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

for(unsigned int i=0; i < collect_input.size(); ++i)
std::cout << collect_input
<< '\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<iostream>
#include<string>
#include<vector>
#include<algorithm>

int main()
{
std::vector<std::string> collect_input;

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

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

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

// sorting the input
sort(collect_input.begin(), collect_input.end());

for(unsigned int i=0; i < collect_input.size(); ++i)
std::cout << collect_input
<< '\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]

// sorting the input
sort(collect_input.begin(), collect_input.end());

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" ?

 

[TB]

arnuld skrev:

// sorting the input
sort(collect_input.begin(), collect_input.end());

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" ?

Because 'Q' = 81 and 'a' = 97 according
to unicode and ascii.

 

[sonison.james]

// sorting the input
sort(collect_input.begin(), collect_input.end());

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_iterator< string >(cout, "\n" ) );

return 0;
}

 

[Zeppe]

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<iostream>
#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::string> collect_input;

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

for(int i=0; input_word != "quit"; ++i)
{
collect_input.push_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::string> collect_input;

while(true){
std::string input_word;
std::cin >> input_word;
if(input_word == "quit")
break;
else
collect_input.push_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.size(); ++i)
std::cout << collect_input
<< '\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

 

[Zeppe]

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.]

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<iostream>
#include<string>
#include<vector>

int main()
{
std::vector<std::string> collect_input;

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

for(int i=0; input_word != "quit"; ++i)
{
collect_input.push_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.push_back( input_word );
}

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

for(unsigned int i=0; i < collect_input.size(); ++i)
std::cout << collect_input
<< '\n';

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

std::copy( collect_input.begin(), collect_input.end(),

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]

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.push_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.size(); ++i)
std::cout << collect_input
<< '\n';

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

std::copy( collect_input.begin(), collect_input.end(),

return 0;
}

2 questions:

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

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

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<iostream>
#include<string>
#include<algorithm>
#include<vector>
#include<set>
#include<iterator>

int main()
{
std::string input_word;
std::vector<std::string> collect_input, removed_duplicates;

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


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

// sorting the input
sort(collect_input.begin(), collect_input.end());
unique_copy(collect_input.begin(), collect_input.end(),
back_inserter(removed_duplicates));

for(std::size_t i=0; i < removed_duplicates.size(); ++i)
{
std::cout << removed_duplicates << '\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]

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.]

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

for(unsigned int i=0; i < collect_input.size(); ++i)
std::cout << collect_input
<< '\n';

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

std::copy( collect_input.begin(), collect_input.end(),

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_iterator" ?

No, but the loop itself is.

for ( std::vector<std::string>::iterator 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 << '\n';
}

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

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.

 

[Victor Bazarov]

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

It is a common belief that for some cases (and some compilers) it is
better to declare/define an object before it's going to be used, and
especially if the use is in the loop. IOW, it's better to write

SomeType object;
while (blah) {
object = someexpression; // assignment
somehow_use_that(object); // use
}

than

while (blah) {
SomeType object = someexpression; // intialisation
somehow_use_that(object);
}

(the former case declares the variable in the scope unnecessarily
wide and sooner than the object is actually used).

It is also true that without measuring it's impossible to say for
sure.

V

 

[Ian Collins]

It is a common belief that for some cases (and some compilers) it is
better to declare/define an object before it's going to be used, and
especially if the use is in the loop. IOW, it's better to write

SomeType object;
while (blah) {
object = someexpression; // assignment
somehow_use_that(object); // use
}

than

while (blah) {
SomeType object = someexpression; // intialisation
somehow_use_that(object);
}

(the former case declares the variable in the scope unnecessarily
wide and sooner than the object is actually used).

This appears to contradict your first paragraph.

 

[Victor Bazarov]

This appears to contradict your first paragraph.

Contradict? How? The belief is that the former (declaring outside
the 'while' loop) is better. I am not claiming any side in that
belief. I am not saying "contrary to the belief it's better to do
it that way" (in case you are reading it that way).

V

 

[Ian Collins]

Contradict? How? The belief is that the former (declaring outside
the 'while' loop) is better. I am not claiming any side in that
belief. I am not saying "contrary to the belief it's better to do
it that way" (in case you are reading it that way).

OK, I thought you were expressing a preference for the first form and
then criticising it at the end. I realy should stop interpreting IOW as
Isle Of Wight!

 

[Duane Hebert]

Contradict? How? The belief is that the former (declaring outside
the 'while' loop) is better. I am not claiming any side in that
belief. I am not saying "contrary to the belief it's better to do
it that way" (in case you are reading it that way).

I think the words "unnecessarily" and "sooner than" tend
to show a bias.

 

[Victor Bazarov]

I think the words "unnecessarily" and "sooner than" tend
to show a bias.

So, you're saying that by using those words I exposed my bias
towards one of the sides [of the debate]. Had I omitted them,
would the issue actually have been clearer?

In this particular case the wideness of the scope and earlier
construction of the object (both unnecessary by themselves) are
supposedly outweighed by performance of the code, taken apriori.

If performance weren't involved, the scope *is* unnecessariry
wide and the object's construction *is* too soon. That's not
just my opinion. My opinion, however, is that it is impossible
to decide without actually measuring the performance.

V

 

[arnuld]

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

OK, it means , "std::cout" always copies its input.

No, but the loop itself is.

for ( std::vector<std::string>::iterator 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 << '\n';

}

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


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

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

i got it, you mean it is not a good programme at all. i need to
rewrite.

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

that is what "std::set" does. here is my try with a compile-time error
i am unable to figure out why. i tried eve "std:stream_iterator" but
same error:

-------- PROGRAMME --------

#include<iostream>
#include<string>
#include<algorithm>
#include<set>
#include<iterator>

int main()
{
std::string input_word;
std::set<std::string> s; // collects inputs

while(std::cin >> input_word && input_word != "quit")
{
s.insert(input_word);
}


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

std::copy(s.begin(), s.end(),
ostream_iterator<std::string>(std::cout, '\n');

return 0;
}

----- OUTPUT ---------
[arch@voodo tc++pl]$ g++ -ansi -pedantic -Wall -Wextra -O
ex_5.9-11_modified.cpp
ex_5.9-11_modified.cpp: In function 'int main()':
ex_5.9-11_modified.cpp:30: error: 'ostream_iterator' was not declared
in this scope
ex_5.9-11_modified.cpp:30: error: expected primary-expression before
'>' token
ex_5.9-11_modified.cpp:30: warning: left-hand operand of comma has no
effect
[arch@voodo tc++pl]$

 

[red floyd]

that is what "std::set" does. here is my try with a compile-time error
i am unable to figure out why. i tried eve "std:stream_iterator" but
same error:

-------- PROGRAMME --------

#include<iostream>

#include<string>
#include<algorithm>
#include<set>
#include<iterator>

int main()
{
std::string input_word;
std::set<std::string> s; // collects inputs

while(std::cin >> input_word && input_word != "quit")
{
s.insert(input_word);
}


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

std::copy(s.begin(), s.end(),
ostream_iterator<std::string>(std::cout, '\n');

return 0;
}

----- OUTPUT ---------
[arch@voodo tc++pl]$ g++ -ansi -pedantic -Wall -Wextra -O
ex_5.9-11_modified.cpp
ex_5.9-11_modified.cpp: In function 'int main()':
ex_5.9-11_modified.cpp:30: error: 'ostream_iterator' was not declared
in this scope
ex_5.9-11_modified.cpp:30: error: expected primary-expression before
'>' token
ex_5.9-11_modified.cpp:30: warning: left-hand operand of comma has no
effect
[arch@voodo tc++pl]$

 

[red floyd]

Ignore my previous post. You forgot std:: on ostream_iterator. Also,
missing a close paren on std::copy. See embedded.

-------- PROGRAMME --------

#include<iostream>
#include<string>
#include<algorithm>
#include<set>
#include<iterator>

int main()
{
std::string input_word;
std::set<std::string> s; // collects inputs

while(std::cin >> input_word && input_word != "quit")
{
s.insert(input_word);
}


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

std::copy(s.begin(), s.end(),
ostream_iterator<std::string>(std::cout, '\n');

std:stream_iterator<std::string(std::cout, '\n'));