-1
\$\begingroup\$

What do you think?

#include <algorithm>

#include <type_traits>

namespace
{

template <typename C, typename = std::size_t>
struct has_find: std::false_type
{
};

template <typename C>
struct has_find<C,
  decltype(
    sizeof(
      (typename C::iterator(C::*)(
        typename C::key_type const&))(&C::find)
    ) |
    sizeof(
      (typename C::const_iterator(C::*)(
        typename C::key_type const&) const)(&C::find)
    )
  )
> : std::true_type
{
};

}

template <class Container, class Key, typename F>
inline std::enable_if_t<!has_find<Container>{}>
find(Container& c, Key const& k, F&& f) noexcept
{
  f(std::find(c.begin(), c.end(), k));
}

template <class Container, class Key, typename F>
inline std::enable_if_t<has_find<Container>{}>
find(Container& c, Key const& k, F&& f) noexcept
{
  f(c.find(k));
}

Usage:

find(container, key, [&](auto const i){ std::cout << "found it" << std::endl; });
\$\endgroup\$
  • \$\begingroup\$ So you decided to re-write std::find_if() \$\endgroup\$ – Martin York Sep 2 '17 at 16:33
  • \$\begingroup\$ @LokiAstari I don't think find_if works on std::map and it's not exactly find_if either. \$\endgroup\$ – user1095108 Sep 2 '17 at 16:34
  • \$\begingroup\$ find_if() works on any container that supports iterators (which is all containers). \$\endgroup\$ – Martin York Sep 2 '17 at 16:35
  • \$\begingroup\$ @LokiAstari well, it's not exactly the same lol :) The predicate will be called for each element in that case, for one thing. \$\endgroup\$ – user1095108 Sep 2 '17 at 16:39
  • \$\begingroup\$ OK. I see that now. It should be called find_first_and_execute() or if you were me findFirstAndExecute() \$\endgroup\$ – Martin York Sep 2 '17 at 16:41
1
\$\begingroup\$
namespace
{
    /* implementation details */
}

Do you really need the level of protection an anonymous namespace provides? Prefer using a named namespace (private, detail, etc) to protect against accidental usage. Use anonymous namespaces for details that need to be limited to the local translation unit.


  template <class Container, class Key, typename F>
  inline std::enable_if_t<!has_find<Container>{}>
  find(Container& c, Key const& k, F&& f) noexcept
  {
    f(std::find(c.begin(), c.end(), k));
  }

When programming generically, prefer calling the non-member functions. Member begin and end do not exist for all containers. However, the standard containers and arrays are supported by non-member begin, end, and the const/reverse variants.

How does the noexcept specification work if f() can throw?

It's not clear why you are applying a function to a find result. Consider writing a wrapper (find_and_apply) on a generic non-member find to apply the result to a function.

Using Expression SFINAE (ymmv on MSVC support), you can simply check if the expression c.find(key) is a well-formed expression. If the expression is ill-formed, the overloaded function is culled from the overload set.

namespace detail {
  template <class Container, class Key>
  auto find(Container& c, const Key& key, int) -> decltype(c.find(key)) {
    return c.find(key);
  }

  using std::begin; // adl
  using std::end;

  template <class Container, class Key>
  auto find(Container& c, const Key& key, char) -> decltype(begin(c)) {
    return std::find(begin(c), end(c), key);
  }
}

template <class Container, class Key>
auto find(Container& c, const Key& key) -> decltype(detail::find(c, key, 0)) {
  return detail::find(c, key, 0);
}

Note: The third parameter in the detail functions utilizes integral types for priority tag dispatching.

\$\endgroup\$
  • \$\begingroup\$ This is good advice that expands on the original idea. I didn't envision supporting arrays :) \$\endgroup\$ – user1095108 Sep 3 '17 at 6:28
3
\$\begingroup\$

From the first glance I see 2 problems:

Item1. This code fails to compile with the highest warning level

src/test.cpp:21:47: error: use of old-style cast [-Werror=old-style-cast]
         typename C::key_type const&))(&C::find)
                                               ^
src/test.cpp:25:53: error: use of old-style cast [-Werror=old-style-cast]
         typename C::key_type const&) const)(&C::find)

Item2. Even if element is not found - predicate is called with end() iterator. Maybe it is better not to change the find interface and just allow to call container find when available and std::find otherwise?

\$\endgroup\$
  • \$\begingroup\$ but that's the whole idea, you can see if key is found by checking the iterator. Usually you want to know, if it is not found, as that is always possible. \$\endgroup\$ – user1095108 Sep 2 '17 at 16:36
  • \$\begingroup\$ @user1095108: The point is you are calling f() on the end() iterator. Which is one past the end. So there is no data element. That is why the find interface returns an iterator to see if you actually found something you need to compare against end(). Its also why you are getting compiler errors. \$\endgroup\$ – Martin York Sep 2 '17 at 16:39
  • \$\begingroup\$ @LokiAstari it's usually a warning, he just set the compiler to the highest warning level. \$\endgroup\$ – user1095108 Sep 2 '17 at 16:48
  • \$\begingroup\$ @user1095108: A warning is a logical error in the code. You should always compile with the warnings turned up and the warnings treated as errors. Sorry. To a beginner warning sounds like its not important. But it just means you are doing something wrong. \$\endgroup\$ – Martin York Sep 2 '17 at 16:55
  • \$\begingroup\$ @LokiAstari There are such things as false warnings. But they are reasonably rare, and nearly always there's a way to clarify the code and thus avoid the warning. \$\endgroup\$ – Deduplicator Sep 2 '17 at 16:59
3
\$\begingroup\$

Well, your has_find is far too over-specified.
Take a look at the docs for std::map.find, and you will find support for other key-types.

Also you should avoid using c-style-casts, as they are far too versatile and thus fail to make your intent manifest.

// From C++17:
template<typename... Ts> struct make_void { typedef void type;};
template<typename... Ts> using void_t = typename make_void<Ts...>::type;

template <class T, class K, class = void>
struct has_find : std::false_type {};
template <class T, class K, class U = void_t<decltype(
    std::declval<typename T::const_iterator&>() = std::declval<T&>().find(std::declval<K&>())
)>>
struct has_find<T, K, U> : std::true_type {};

Next, your find()'s contract and prototype are surprising. Nobody expects it to call the supplied callable with the result of trying to find the element.

If you decide to do so anyway, call it something more like find_and_process(), and don't execute the callback with an end-iterator.

\$\endgroup\$
  • \$\begingroup\$ The idea was to avoid having to write code initializing a temporary variable like auto const i(map.find("blabla")) then checking for end() and then doing something with i, if key was or wasn't found. \$\endgroup\$ – user1095108 Sep 2 '17 at 17:35
  • \$\begingroup\$ Well, iterators are generally cheaply, if not trivially, copyable. So, don't worry about that. \$\endgroup\$ – Deduplicator Sep 2 '17 at 17:39
  • \$\begingroup\$ I wasn't worried, just the boilerplate code bothered me. \$\endgroup\$ – user1095108 Sep 2 '17 at 17:40
0
\$\begingroup\$

Suggestion - use convenience functions for common operations.

template<class Container, class Key>
bool exists(Container const& container, Key const& key)
{
    bool returnValue = false;
    find(container, key, [&](auto const i)
    {
        returnValue = (i != end(container));
    });

    return returnValue;
}

int main()
{
    std::vector<int> container = {1,2,3,4,5};
    int key = 7;

    // Ah, less typing
    if (exists(container, key))
        std::cout << "found it" << std::endl;
    else
        std::cout << "not found" << std::endl;

    // Than this lambda stuff
    find(container, key, [&](auto const i)
    {
        if (i != container.end())
            std::cout << "found it" << std::endl;
        else
            std::cout << "not found" << std::endl;
    });
}

And for mutating the value, maybe something like...

ValueType* lookup(Container& container, Key const& key);

...

if (auto v = lookup(container, key))
    *v = 12;
\$\endgroup\$
  • \$\begingroup\$ nice try mercury, but you need to practice writing c++ hieroglyphics some more :) \$\endgroup\$ – user1095108 Sep 2 '17 at 21:27
0
\$\begingroup\$

I went with this. It supports arrays, maps and vectors. Changed the name and added the appropriate noexcepts.

namespace detail
{

template <class Container, class Key>
inline auto find(Container& c, Key const& k, int)
  noexcept(noexcept(c.find(k))) -> decltype(c.find(k))
{
  return c.find(k);
}

template <class Container, class Key>
inline auto find(Container& c, Key const& k, char)
  noexcept(noexcept(std::find(std::begin(c), std::end(c), k))) ->
  decltype(std::find(std::begin(c), std::end(c), k))
{
  return std::find(std::begin(c), std::end(c), k);
}

}

template <class Container, class Key>
inline auto any_find(Container& c, Key const& k) noexcept(
  noexcept(detail::find(c, k, 0))
)
{
  return detail::find(c, k, 0);
}

template <class Container, class Key, typename F>
inline void any_find(Container& c, Key const& k, F&& f) noexcept(
  noexcept(f(detail::find(c, k, 0)))
)
{
  f(detail::find(c, k, 0));
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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