Most C++'ish way to check if value belongs to certain static set

Question

Let's say that I want to write something like this (the {1, 3, 7, 42, 69, 550123} set is known before compilation):

int x;
...
if (x == 1 || x == 3 || x == 7 || x == 42 || x == 69 || x == 5550123)
{
  ...
}

The condition looks ugly because we have 9 extra symbols ("|| x ==") for each possible value. How can I rewrite it in a more C++'ish way?

My best guess is:

int x;
...
const std::unordered_set<int> v = {1, 3, 7, 42, 69, 5550123};
if (v.count(x))
{
  ...
}

It has O(1) average complexity with some memory and time overhead, but still looks kinda ugly.

Acrogen

en.cppreference.com/w/cpp/algorithm/all_any_none_of – Cantina

I don't see what you find ugly about the second option, but I would like to add that, given these numbers are known, perhaps a const vector with ordered values could be better, then you can use bisection if you set grows large, this will give you O(log N). – Hubby

That's what the switch statement is for. – Adaurd

I would say the above is quite idiomatic, even if the semantics of checking for existence is of an element using count(...) > 0 is not as good as e.g. contains(...), which will be available in C++20. – Garald

The big O notation describes asymptotic behavior; it is meaningless for a fixed or small input size. – Flautist

if (x <in> {1, 3, 7, 42, 69, 5550123}) good enough? github.com/klmr/named-operator – Telepathy

Don't use unordereded_map/vector for this. They are highly inefficient for this task (if you have only 6 numbers). – Pluralize

I think with boost Hana you can do this very easily and most performant. – Lauritz

Answer 1

Edit: I just noticed c++14 tag. Note that my implementation of in relies on C++17. It can be done in C++14 as well using recursion, but that involves much more boilerplate, and a bit slower compilation.

One could use a template to generate a function with a logical operator sequence, such as the one in nvoigt's answer:

template<auto... ts, class T>
constexpr bool
in(const T& t) noexcept(noexcept(((t == ts) || ...))) {
    return ((t == ts) || ...);
}

// usage
if (in<1, 3, 7, 42, 69, 5550123>(x))

That said, hiding the set of magic numbers behind a named function probably makes a lot of sense:

constexpr bool
is_magical(int x) noexcept {
    return in<1, 3, 7, 42, 69, 5550123>(x);
}

Erasme

Am I right that the return value of "is_magical" will be pre-computed during compilation if I compile it with -O2? – Acrogen

@Acrogen edit: The result can only be pre-computed if x is known at compile time, and the function was expanded inline. -O2 does enable inline-small-functions, but not inline-functions, and I don't know when a function is considered small. – Erasme

constexpr might be added to function. – Thor

OP had not mentioned C++14 in the question body, so, given how multiple answers (also) suggest C++17 solutions - I've decided to remove the tag. Also, would appreciate feedback on my approach :-) – Psychoneurotic

@Psychoneurotic Feedback: It uses PP macro :( The syntax is unconventional. It isn't constexpr. It uses std::initializer_list which causes copies which could be an issue with complex types. Other than that, I see no problem. – Erasme

@eerorika: 1. Macros are part of C++. 2. Most solutions here, including your own, suggest syntax that is unconventional in some ways... I went for straightforward and terse rather than obviousness-of-implementation . 3. Can you be more specific about the dangers of copying with std::initializer_list ? I'll double-check my use of references though. 4. Add the missing constexpr. – Psychoneurotic

@Psychoneurotic 1. Yes, and they are very problematic. It's best to avoid them when possible, and preferably limit them to the pattern of: define, use, undefine. And follow the common naming conventions. 2. I don't agree in<1, 3, 7, 42, 69, 5550123>(x) being unconventional syntax. It's just a function call with explicit template arguments. is_magical(x) is even less so. 3. Actually, I take it back. The problem I was thinking of was when storing the elements of initialiser list elsewhere; but we aren't storing but rather comparing only. So that should be fine. – Erasme

Answer 2

The only clean way to do this is to simply move it to a method. Name the method appropriately and it really does not matter what you do inside.

bool is_valid_foo_number(int x)
{
    return x == 1
        || x == 3
        || x == 7
        || x == 42
        || x == 69
        || x == 5550123;
}

The method looks good enough for me, because all I will ever see of it is

if (is_valid_foo_number(input))
{
    // ...
}

Should a technical detail change (like the sheer amount of valid numbers requiring another lookup approach or maybe a database instead of hardcoded values) you change the method's internals.

The point is that I think it only looks ugly... because you need to look at it while you look at your logic. You shouldn't have to look at the details anyway.

Urbano

Yeah if we're talking about a huge project then you're probably right, proper formatting and function separation does better than advanced functions and constructions. – Acrogen

Answer 3

How can I rewrite it in a more C++ way?

Suggestion: make a variadic template function for it, make it generic (non only for int values) and make it constexpr; this way you can check, the presence of the value in the set, compile time.

You tagged C++14 but I show first the recursive way for C++11, with a recursive case and a ground case

template <typename T>
constexpr bool is_in_list_11 (T const &)
 { return false; }

template <typename T, T t0, T ... ts>
constexpr bool is_in_list_11 (T const & t)
 { return (t == t0) || is_in_list_11<T, ts...>(t); }

Starting from C++14, a constexpr function can be a lot more complex, so recursion isn't necessary anymore and you can write something as

template <typename T, T ... ts>
constexpr auto is_in_list_14 (T const & t)
 {
   using unused = bool[];

   bool ret { false };

   (void)unused { false, ret |= t == ts ... };

   return ret;
 }

Starting from C++17 you can use also auto template type and template folding, so (as user2079303 showed before) the function become very very simple

template <auto ... ts, typename T>
constexpr auto is_in_list_17 (T const & t)
 { return ( (t == ts) || ... ); }

The following is a full working example with all versions

#include <iostream>

template <typename T>
constexpr bool is_in_list_11 (T const &)
 { return false; }

template <typename T, T t0, T ... ts>
constexpr bool is_in_list_11 (T const & t)
 { return (t == t0) || is_in_list_11<T, ts...>(t); }

template <typename T, T ... ts>
constexpr auto is_in_list_14 (T const & t)
 {
   using unused = bool[];

   bool ret { false };

   (void)unused { false, ret |= t == ts ... };

   return ret;
 }

template <auto ... ts, typename T>
constexpr auto is_in_list_17 (T const & t)
 { return ( (t == ts) || ... ); }

int main ()
 {
   constexpr auto b11a { is_in_list_11<int, 1, 3, 7, 42, 69, 5550123>(7) };
   constexpr auto b11b { is_in_list_11<int, 1, 3, 7, 42, 69, 5550123>(8) };

   constexpr auto b14a { is_in_list_14<int, 1, 3, 7, 42, 69, 5550123>(7) };
   constexpr auto b14b { is_in_list_14<int, 1, 3, 7, 42, 69, 5550123>(8) };

   constexpr auto b17a { is_in_list_17<1, 3, 7, 42, 69, 5550123>(7) };
   constexpr auto b17b { is_in_list_17<1, 3, 7, 42, 69, 5550123>(8) };

   std::cout << b11a << ' ' << b11b << std::endl;
   std::cout << b14a << ' ' << b14b << std::endl;
   std::cout << b17a << ' ' << b17b << std::endl;
 }

Ibarra

Answer 4

Here's my suggestion. It lets you write:

int x;
// ...
if ( x is_one_of {1, 3, 7, 42, 69, 550123} ) {
    // ...
}    

which is just about the most straightforward way you could think of.

... but - what dark magick be this? Surely I must be cheating, right?

Well, no, no cheating; or at least, not really cheating. This solution is the "most C++'-ish" in the sense that, employing multiple hacks including ugly ones, C++ lets you customize its effective syntax much more than you would expect!

Here's the implementation, in C++17:

#include <initializer_list>

namespace detail {

struct is_one_of_op {};

template <typename T>
struct is_one_of_op_primed { 
    T&& t; 

    template <typename... Us>
    constexpr bool operator+(std::initializer_list<Us...>&& list) {
        for(const auto& element : list) {
            if (t == element) { return true; }
        }
        return false;
    }
};

template <typename T>
constexpr is_one_of_op_primed<T> operator+(T&& t, is_one_of_op)
{
    return is_one_of_op_primed<T>{std::forward<T>(t)};
}

} // namespace detail

#define is_one_of + detail::is_one_of_op{} + std::initializer_list

See it in action: GodBolt

Notes:

• The inspiration for this implementation is partially due to this answer here on SO.
• Caveat: The values must all have the same type or bad things will happen >:-)

Psychoneurotic

Answer 5

Try this:

#include <iostream>
#include <vector>
#include <algorithm>
int main()
{
  std::vector<int> v = {1, 3, 7, 42, 69, 5550123};
  auto is_present = [&v](int x)->bool{
      return  std::find(v.begin(),v.end(),x) != v.end();
  };      
  std::cout << (is_present(1)? "present" :" no present") <<std::endl;       
}

Clipper

std::find or std::binary_search can be used instead of hand rolled loop. – Thor

@Clipper you can use std::array instead of vector as length and numbers are known. – Baste

if (expr) return true; return false; is equivalent to return expr; – Disability

You probably want to use an std::array instead of std::vector for a fixed set of items. – Psychoneurotic

Answer 6

As the other answers say, move this to a function.

As the other answers say, you can consider adding constexpr / throw as required.

As the other answers don't say, use a switch case statement for this; which allows you to replace all the || x == with case - a few characters less, which might not seem significant (and it kinda isn't); but most importantly removes the chance of a mistake with either the variable name or a |.

When you've 300 items in this list, and it doesn't work as expected, trust me, you'll be glad to not have to check that every || is correct.

Cliftonclim