Jeff Schwab
11/7/2008 6:14:00 PM
Jeff Schwab wrote:
> Juha Nieminen wrote:
>> Jeff Schwab wrote:
>>> template<typename T>
>>> std::ostream& operator<<(std::ostream& out, std::vector<T> const& v) {
>>> if (!v.empty()) {
>>> typedef std::ostream_iterator<T> out_iter;
>>> copy(v.begin(), v.end() - 1, out_iter( out, " " ));
>>> out << v.back();
>>> }
>>> return out;
>>> }
>>
>> Is there some advantage of that code over a shorter and simpler:
>>
>> template<typename T>
>> std::ostream& operator<<(std::ostream& out, std::vector<T> const& v) {
>> for(std::size_t i = 0; i < v.size()-1; ++i)
>> out << v[i] << " ";
>> out << v.back();
>> return out;
>> }
>
> The use of the standard algorithm (rather than a hand-rolled loop) helps
> separate the different levels of abstraction. In your example, the code
> that works with an individual datum of type T is all mixed up with the
> code that works with the vector as a whole. There are decent
> discussions of this in Effective STL (Scott Meyers) and Clean Code
> (Robert Martin).
You know, the code I posted has exactly the same problem I pointed out
in yours: the level of abstraction is mixed. I was imagining the work
split neatly between two standard utilities:
1. std::copy handles iteration through the vector.
2. std::ostream_iterator handles the output of each element.
The problem is that the last item should be output slightly differently
from the others, so the code can't be factored quite so neatly with
those utilities. The semantic messiness is reflected in the code: the
explicit check for an empty vector (which should be irrelevant at that
level), the hard-coded -1, and the call of v.back() are all symptoms.
The worst issue is the manual output of the last vector element, which
should be the ostream_iterator's job. You could improve the situation a
little by delegating to another ostream_iterator:
*out_iterator( out ) = v.back();
You would still be doing the copy algorithm's job, though. You could
avoid that by calling copy twice, e.g:
copy(v.begin(), v.end()-1, out_iter( out, " " ));
copy(v.end()-1, v.end(), out_iter( out ));
That's silly, though, because it implies a loop over just one element.
You don't need the full copy algorithm; it's just that std::copy is the
closest thing to what we need that happens to be in the standard library.
What's really needed is a set of utilities that better fit the natural
division of the problem. The following seems to me like a reasonable
solution: Use a custom (but generic) copy algorithm that implicitly
gives the last element special treatment, and use two separate output
iterators:
#include <algorithm>
#include <iostream>
#include <iterator>
#include <vector>
/* Assign all but the last element of [pos, end) to *out, and assign
* the last element to *fin. */
template<typename Fwd_iter, typename Out_iter>
void final_copy(
Fwd_iter pos, Fwd_iter end,
Out_iter out, Out_iter fin) {
if (pos != end) {
for (Fwd_iter next = pos; ++next != end; pos = next) {
*out = *pos;
++out;
}
*fin = *pos;
++fin;
}
}
template<typename T>
std::ostream& operator<<(
std::ostream& out, std::vector<T> const& v) {
typedef std::ostream_iterator<T> out_iter;
final_copy(
v.begin(), v.end(),
out_iter( out, " " ),
out_iter( out ));
return out;
}
int main() {
int const ints[] = { 1, 2, 3, 4 };
std::cout << std::vector<int>( ints, ints + 4 ) << '\n';
}
The output operator for the vector still has two responsibilities:
Configuration/composition of the utilities to perform the actual output,
and implementation of the standard conventions for operator<<, e.g.
returning the output stream by reference. It probably should be further
refactored along those lines:
template<typename T>
void print(std::ostream& out, std::vector<T> const& v) {
typedef std::ostream_iterator<T> out_iter;
final_copy(
v.begin(), v.end(),
out_iter( out, " " ),
out_iter( out ));
}
template<typename T>
std::ostream& operator<<(
std::ostream& out, std::vector<T> const& v) {
print(out, v);
return out;
}
The only thing left that I don't like is the configuration code embedded
in the print function, specifically the hard-coded " ". The best
solution (for non-trivial applications) might be a traits class that
tells what the separator should be between elements, and defaults to " "
or "\n":
out_iterator( out, separator<T>::value );