Return to Revisions

Does not compile unless I add the move operators to integer.

struct integer
{
 int x;
 integer(int y): 
 x(y)
 {} 
 integer(const integer& other) = delete; //non copyable
 integer& operator=(const integer& other) = delete;
 // Added these
 integer(integer&&) = default;
 integer& operator=(integer&&) = default;
};

You don't need that lamda in the your main function. You just did not specify your comparison operator correctly.

return most_frequent(first, last, std::less<>{});
// Should be
return most_frequent(first, last, std::less<typename std::iterator_traits<ForwardIterator>::value_type>{});

Then you can remove:

auto comp = [&comparator](const auto& lhs, const auto& rhs)
{
 return comparator(lhs.get(), rhs.get());
};

and just use Comparator where you use comp

There is no need to have two versions of the function most_frequent just use default parameter values.

// This is not needed
// You can remove it.
template <typename ForwardIterator>
std::pair<ForwardIterator, std::size_t> most_frequent(ForwardIterator first, ForwardIterator last)
{
 return most_frequent(first, last, std::less<>{});
}

Modify the declaration of the main function:

template <typename ForwardIterator, typename Comparator = std::less<typename std::iterator_traits<ForwardIterator>::value_type>>
 // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
std::pair<ForwardIterator, std::size_t> most_frequent(
 ForwardIterator first,
 ForwardIterator last,
 Comparator comparator = Comparator())
 // ^^^^^^^^^^^^

The code executes in time faster than human reaction, so I think for the first version this should be enough.

I would hope so.
Your tests sets are relatively small. Haw long does it take to scan the whole library of congress would be a better test.

Final Version:

template <typename ForwardIterator, typename Comparator = std::less<typename std::iterator_traits<ForwardIterator>::value_type>>
std::pair<ForwardIterator, std::size_t> most_frequent(ForwardIterator first,
 ForwardIterator last, Comparator comparator = Comparator())
{ 
 std::map<std::reference_wrapper<typename std::iterator_traits<ForwardIterator>::value_type>,
 std::size_t, Comparator> counts(comparator);
 
 std::size_t frequency = 0;
 auto most_freq = first;
 while (first != last)
 { 
 std::size_t current = ++counts[*first];
 if (current > frequency)
 { 
 frequency = current;
 most_freq = first;
 }
 
 ++first;
 }
 
 return std::make_pair(most_freq, frequency);
}