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::stringcollect_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[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<iostream>
#include<string>
#include<vector>
#include<algorithm>

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.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[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" <[e-mail]:

i just did not get 2 things:

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

[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<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::stringcollect_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::stringcollect_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[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" <[e-mail]:

On Apr 9, 2:07 pm, "arnuld" <[e-mail]:

i just did not get 2 things:

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

arnuld wrote:

>On Apr 9, 2:07 pm, "arnuld" <[e-mail]:

i just did not get 2 things:

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

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*********@gmail.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<iostream>
#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.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[i]
<< '\n';

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

std::copy( collect_input.begin(), collect_input.end(),
ostream_iterator<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." <[e-mail]:

"arnuld" <[e-mail]:

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[i]
<< '\n';

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

std::copy( collect_input.begin(), collect_input.end(),
ostream_iterator<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_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::stringcollect_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[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]:

>
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*********@gmail.comwrote:

On Apr 9, 4:38 pm, "Daniel T." <[e-mail]:
"arnuld" <[e-mail]:

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

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

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

std::copy( collect_input.begin(), collect_input.end(),
ostream_iterator<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_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[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.

[Victor Bazarov]

Anand Hariharan wrote:

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

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

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
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Ian Collins]

Victor Bazarov wrote:

Anand Hariharan wrote:

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

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

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.

--
Ian Collins.

[Victor Bazarov]

Ian Collins wrote:

Victor Bazarov wrote:

>Anand Hariharan wrote:

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

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?

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.

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
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Ian Collins]

Victor Bazarov wrote:

Ian Collins wrote:

>>Victor Bazarov wrote:

>>>Anand Hariharan wrote:

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

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

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.

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!

--
Ian Collins.

[Duane Hebert]

"Victor Bazarov" <v.********@comAcast.netwrote in message
news:ev**********@news.datemas.de...

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

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.

[TB]

arnuld skrev:

>On Apr 9, 2:07 pm, "arnuld" <[e-mail]:

i just did not get 2 things:

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

--
TB

[Victor Bazarov]

Duane Hebert wrote:

"Victor Bazarov" <v.********@comAcast.netwrote in message
news:ev**********@news.datemas.de...

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

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.

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
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[arnuld]

On Apr 10, 1:56 am, "Daniel T." <[e-mail]:

"arnuld" <[e-mail]:

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.

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

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[i] << '\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.

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::ostream_iterator" but
same error:

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

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

int main()
{
std::string input_word;
std::set<std::strings; // 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]

arnuld wrote:

>
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::ostream_iterator" but
same error:

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

#include<iostream>

#include <ostream>

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

int main()
{
std::string input_word;
std::set<std::strings; // 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.

arnuld wrote:

>
-------- PROGRAMME --------

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

int main()
{
std::string input_word;
std::set<std::strings; // 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::ostream_iterator<std::string(std::cout, '\n'));

>
return 0;
}

[red floyd]

red floyd wrote:

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

And the second param to ostream_iterator constructor is a const char *,
not a char, so it should be "\n"

>
arnuld wrote:

>-------- PROGRAMME --------

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

int main()
{
std::string input_word;
std::set<std::strings; // 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::ostream_iterator<std::string>(std::cout, '\n'));

> return 0;
}

[arnuld]

On Apr 10, 11:05 am, red floyd <[e-mail]:

red floyd wrote:

And the second param to ostream_iterator constructor is a const char *,
not a char, so it should be "\n"

YES, it works now. i got it:

1.) 1st parameter to "ostream_iterator" needs to be the stream where i
am sending the output.

2.) 2nd parameter to "ostream_iterator" must be a string literal.
now from other posts, i conclude:

3.) there are 2 benefits of "std::set" container:

i.) it keeps the items sorted.
ii.) it removes duplication of elements.

4.) "std::copy" container is a generic one and is for maintainability.
e.g if we want to change/modify or make some additions to our
programme like if we want to use a set or list rather than a vector.

thanks

[Zeppe]

Victor Bazarov wrote:

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.

Exactly, if the construction of the object is not that though and if the
loop is not too critic, there is often no point in avoid the reiteration
of the construction of the object.

Also, the construction inside the loop often allows you to obey the RAI
paradigm that is not a bad thing.

Actually, in many cases automatic optimization could be done but I don't
know if the compiler is "smart" enough to determine when the constructor
of an object can be called just once instead that in each loop. I don't
think so.

Regards,

Zeppe

[Duane Hebert]

"Victor Bazarov" <v.********@comAcast.netwrote in message
news:lI******************************@comcast.com. ..

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

No I was just commenting on why you got the reply
that you did. I should have appended a <g>.

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.

Sure.

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.

I would tend to use the more narrow scope and later
construction unless profiling showed it to be a bottleneck.
I've never had this be the case though.

[James Kanze]

On Apr 9, 12:34 pm, Zeppe <[e-mail]:

arnuld wrote:

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

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

I've also seen a tab there:-).

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

What displeases me the most here is that he declares an
increments a variable that is never used. It makes me think
that he's forgotten something.

There's also the slight problem that if the user forgets to put
"quit" in the input file, he ends up in an endless loop.

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.push_back(input_word);
}

Which is worse than his original, lying as it does in the
condition, and then hiding a break deap down where no one can
find it. In this case, there is a very easy and idiomatic
solution:

std::vector< std::string collect_input ;
std::string word ;
while ( std::cin >word && word != "quit" ) {
collect_input.push_back( 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.

Even in small programs like this, it's important to handle
incorrect input. (In practice, you'd probably want to make the
check for "quit" case insensitive, but that's not in the
requirements specification for now, and implementing a case
insensitive compare function is not a job for a beginner.)

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.

The general rule is never to declare a variable until you can
initialize it. Regretfully, the rule doesn't work where input
is involved.

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.

I'm not sure I follow. In any real code, I'd split the input
and output out into separate functions, so that the variable
would not be available outside of the function. And since
whether you succeed in reading it is one of the loop conditions,
and you cannot read it unless it has been previously declared,
your stuck declaring it outside the loop.

It's no big deal. The only thing that is a bit bothersome is
having to declare it without a valid initial value, but there's
no way around that.

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

for(unsigned int i=0; i < collect_input.size(); ++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.

In this case, no. If you do have to break it into separate
lines, I'd align the << characters, to facilitate reading.

Also, I'd generally go for std::endl instead of '\n', especially
in beginner's code.

And of course, you just aren't "in" unless you use iterators
instead of indexes:-).

Also, pay attention with the
for without parenthesis,

You mean braces, I think. ("{}", and not "()".)

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.

I think it's largely a question of conventions. If the opening
brace is on the same line as the for/if/while (as he's doing),
it's generally a good idea to systematically use braces, since
the opening brace (or its absense) is easily overlooked. If the
opening brace is on a separate line, I have no real problem with
dropping the braces.

The important thing is to chose one style, and use it
consistently.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[James Kanze]

On Apr 10, 12:12 pm, "Duane Hebert" <[e-mail]:

I would tend to use the more narrow scope and later
construction unless profiling showed it to be a bottleneck.
I've never had this be the case though.

And measure after as well. The one time I actually measured, it
turned out that the narrower scope was faster---in the case in
question, that the copy constructor was faster than the
assignment operator.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[James Kanze]

On Apr 9, 1:38 pm, "Daniel T." <[e-mail]:

"arnuld" <[e-mail]:

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

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

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

There's such a thing as a bad idiom. The ostream_iterator will
actually work here, but it doesn't in general; it doesn't give
enough control over the output format. (In general, it doesn't
allow using a separator, rather than a terminator. And try to
use it to get a right-aligned column of int's.) It's of little
enough use that I would not recommend wasting the time to learn
it.

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.

It will also return 0 even if there is an error. Not a big deal
for a learning program, but it's never to late to develop good
habits:

std::cout.flush() ;
return std::cout ? EXIT_SUCCESS : EXIT_FAILURE ;

(In production code, of course, one would want an error message,
and not just a change in the return code.)

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[James Kanze]

On Apr 10, 6:11 am, "arnuld" <[e-mail]:

On Apr 10, 1:56 am, "Daniel T." <[e-mail]:

[...]

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

(I'm sure he meant "exercise", but it's an interesting slip.)

that is what "std::set" does.

No it's not. "std::set" keeps things sorted.

I think his point about the hard part is that it requires some
additional checking. Something like:

while ( std::cin >word && word != "quit" ) {
if ( /* word not yet seen */ ) {
collector.push_back( word ) ;
}
}

There are several way of implementing the part in comments: the
simplest is probably to use std::find on the vector, but that
becomes slow when the number of words becomes large.
Alternatively, you keep the words in both a set and a vector,
and only insert when you don't find it in the vector.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]