Stroustrup 5.9 exercise 11

[red floyd]

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]

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]

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]

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

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]

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.

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
<< '\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]

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]

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

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]

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