Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm just curious if this is clear to the average person.

template<typename IteratorType>
inline IteratorType skip_over(
    IteratorType begin,
    IteratorType end,
    typename std::iterator_traits<IteratorType>::value_type skippedCharacter)
{
    typedef typename std::iterator_traits<IteratorType>::value_type value_type;
    return std::find_if(begin, end, 
            std::not1(
                 std::bind2nd(std::equal_to<value_type>(), skippedCharacter)
            )
        );
}
share|improve this question
    
It took me a while to work out what it did (comments would not go amiss). –  Loki Astari Aug 3 '11 at 3:56

1 Answer 1

A few comments:

  • Algorithms typically use the names first and last for the iterators they take, not begin and end (in common usage, begin and end refer specifically to the iterators that delimit a range in a container).

  • The use of find_if seems a bit excessive: yes, it is good to use the Standard Library algorithms, but if you are writing your own algorithm, you may as well just write a loop, especially if it makes the code much clearer.

  • With respect to template parameter naming, it is helpful if you say what category of iterator is required; this helps to document the algorithm.

Consider the following, alternative implementation:

template <typename ForwardIterator, typename T>
ForwardIterator skip_over(ForwardIterator first, ForwardIterator last, T const& x)
{
    while (first != last && *first == x)
        ++first;

    return first;
}
share|improve this answer
    
+1: All good points. –  Loki Astari Aug 3 '11 at 3:55
1  
I'm not sure why, but I think I'd prefer for (;first!=last && *first==x; ++first); return first; (with the empty semicolon on its own line, of course). –  Jerry Coffin Aug 4 '11 at 22:10

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.