Reputation: 83
I've been diving into C++ lately, and I've been giving STL a go. I'm encountering a problem that I cannot seem to solve. I'm guessing it involves me not understanding how C++ deduces types, but I'm not sure. Here is the code that works
template <typename T>
auto most_frequent_element(const std::vector<std::reference_wrapper<T>> &vector) -> boost::optional<std::pair<const std::reference_wrapper<T>, size_t>> {
if (vector.empty()) {
return{};
}
std::map<std::reference_wrapper<T>, size_t> individual_counts = {};
std::for_each(vector.begin(), vector.end(), [&individual_counts](auto &element) {
auto result = individual_counts.emplace(element, 0);
(*result.first).second++;
});
return *std::max_element(individual_counts.begin(), individual_counts.end(), [](auto &a, auto &b) {
return a.second < b.second;
});
}
And this is how I might call it
auto main(int argc, char *argv[]) -> int {
std::vector<char> test = { 'a', 'a', 'b', 'b', 'c'};
auto result = most_frequent_element(std::vector<std::reference_wrapper<char>>(test.begin(), test.end()));
if (result) {
std::cout << (*result).first << " " << (*result).second << std::endl;
}
else {
std::cout << "Empty input list." << std::endl;
}
return EXIT_SUCCESS;
}
So that works. But if I create a simple method to eliminate the need to copy the vector manualy, like so
template <typename T>
auto most_frequent_element_2(const std::vector<T> &vector) -> boost::optional<std::pair<const std::reference_wrapper<T>, size_t>> {
most_frequent_element(std::vector<std::reference_wrapper<T>>(vector.begin(), vector.end()));
}
and call it like this
auto main(int argc, char *argv[]) -> int {
std::vector<char> test = { 'a', 'a', 'b', 'b', 'c'};
auto result = most_frequent_element_2(test);
if (result) {
std::cout << (*result).first << " " << (*result).second << std::endl;
}
else {
std::cout << "Empty input list." << std::endl;
}
return EXIT_SUCCESS;
}
I get the following error
Error C2664 'std::reference_wrapper<char>::reference_wrapper(std::reference_wrapper<char> &&)': cannot convert argument 1 from 'const char' to 'char &'
I'm confused because how I see it they should do the same thing. If I am wrong can someone point me in the right direction.
P.S. I'm working in Visual Studio 2015
Update: Added const constraint to return type of functions to correctly reflect the fact that the *max_element returns a const key. (suggested by @dyp)
Upvotes: 2
Views: 1700
Reputation: 39121
The issues are related to constness. Originally, there's been an issue with the implicit conversion from
optional<pair<const reference_wrapper<T>, size_t>> // A
to
optional<pair<reference_wrapper<T>, size_t>> // B
The first one is const
because the key of std::map
nodes is const
- preventing mutation of the key shall ensure that the tree data structure remains valid.
This implicit conversion is not allowed; this might have to do with the issue described for std::pair
in Improving pair
and tuple
- N4387.
A quick fix is to use A
as the return type, instead of B
. Alternatively, use explicit conversion (a cast or construction of a named object).
The second issue related to constness is that a
reference_wrapper<T>
cannot be initialized with a
T const&
Just like a T&
cannot be initialized from an lvalue-expression of type T const
. This issue occurs because the most_frequent_element_2
function takes its argument by const&
, then the calls to begin
and end
produce const_iterator
s.
A quick and dirty solution is to create a vector<reference_wrapper<T const>>
instead:
most_frequent_element(std::vector<std::reference_wrapper<T const>>(vector.begin(), vector.end()));
// ^^^^^
And then adjust the return type of the _2
function to
boost::optional<std::pair<const std::reference_wrapper<T const>, size_t>>
// ^^^^^
A better solution might be to recognize that the _2
function does not need a vector
, but only two iterators to create a new vector. (The same argument holds for the original most_frequent_element
function.) You could write it instead as:
template <typename InIt,
typename T = std::remove_reference_t<decltype(*std::declval<InIt&>())>>
auto most_frequent_element_2(InIt b, InIt e)
-> boost::optional<std::pair<const std::reference_wrapper<T>, size_t>> {
return most_frequent_element(std::vector<std::reference_wrapper<T>>(b, e));
}
where I've used the second (defaulted) template parameter merely for convenience (a typedef
within the declaration).
Upvotes: 2