Closed justinmeiners closed 2 years ago
Looks good!
Making binary_counter::counter
public for testing is understandable, but I wonder whether there's a cleaner way round the debugging requirement. You could consider providing cbegin
and cend
methods on binary_counter
(returning const_iterator
) to allow callers to iterate over the elements directly. i.e. counter.cbegin()
rather than the uglier counter.counter.begin()
.
show_counter
is essentially a std::copy
into a std::ostream_iterator<char>(std::cout, " ")
. You could call std::copy
directly from main
and avoid needing to write the show_counter
function. On the other hand, having the function raises the level of abstraction in main
. And if you were going to replace the loop in the function with a std::copy
then you'd need to start using std::iterator_traits<I>::value_type
to deduce the underlying element type. On balance I suspect the raw for-loop in a function (as you have it) is cleaner!
Sorry, should have said that this is a really good example of how the binary counter should be used and what it does internally. Would have helped me a lot in my early understanding. Thank you!
public.
I agree it's not great. I was trying to avoid making any modifications to the code, but I think mine is worse since Alex explicitly mentions it being private. I have added begin/end (I believe the cbegin/cend convention is C++11?).
std::copy
I like this better, and chose to remove the function and make it inline.
fix #48 for @aharrison24