Re: Stroustrup 5.9 exercise 11
On Apr 9, 12:34 pm, Zeppe <z...@email.it> wrote:
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::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.
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::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);
}
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:james.kanze@gmail.com
Conseils en informatique orient=E9e objet/
Beratung in objektorientierter Datenverarbeitung
9 place S=E9mard, 78210 St.-Cyr-l'=C9cole, France, +33 (0)1 30 23 00 34