54

Is there a better way to write code like this:

if (var == "first case" or var == "second case" or var == "third case" or ...)

In Python I can write:

if var in ("first case", "second case", "third case", ...)

which also gives me the opportunity to easily pass the list of good options:

good_values = "first case", "second case", "third case"
if var in good_values

This is just an example: the type of var may be different from a string, but I am only interested in alternative (or) comparisons (==). var may be non-const, while the list of options is known at compile time.

Pro bonus:

  • laziness of or
  • compile time loop unrolling
  • easy to extend to other operators than ==
Ruggero Turra
  • 16,929
  • 16
  • 85
  • 141
  • 4
    Well there *is* [`std::any_of`](http://en.cppreference.com/w/cpp/algorithm/all_any_none_of) but it doesn't work on "literal" lists like that. So no there's no straight conversion from the Python expression to a similar C++ expression. Shouldn't be to hard to make a templated `in` function using `std::any_of` and [`std::initializer_list`](http://en.cppreference.com/w/cpp/utility/initializer_list). – Some programmer dude Mar 14 '16 at 09:52
  • 1
    you can write a template function like a `templatebool eq(T what, T a, T b, T c){ return what == a || what == b || what == c; }` and use it: `if(eq(var, "first", "second", "third"))` – nikniknik2016 Mar 14 '16 at 09:53
  • @Ruggero Turra instead of going for a new keywords like any_of, better you can prefer `for()` loop it will make the program readable easily. – Embedded C Mar 14 '16 at 14:17
  • 5
    Compact code is not legible code. "Compactness" is not a virtue in coding. – Almo Mar 14 '16 at 14:18
  • Nitpick: If you did that in python, the extra parentheses were unnecessary. – DJMcMayhem Mar 14 '16 at 15:00
  • 6
    @Almo: I couldn't disagree more. Naturally, there are things so terse (I'm looking at you, regex) that they aren't considered legible by mere mortals but, all other things being equal, less code is always better than more. – Robert Harvey Mar 15 '16 at 00:50
  • While compaction may not be an atrocity itself, is it most definitely not a virtue either! Entering a habit of compacting code, as opposed to improving readability, inevitably leads to countless badly named variables. Naming variables poorly is creating torture for your teammates, and is even considered malicious if done on purpose. Choosing to do something that is known to inevitably lead to the torture of others is clearly not behavior showing high moral standards, thus "Compactness" is not a virtue in coding. – MickLH Mar 15 '16 at 05:19
  • "Less code is always better" in code golf maybe! In any practical application, however, code size savings on the order of a few bytes are deeply frivolous. Readability and conceptual complexity together directly determine, per individual, how easily a piece of code is understood and maintained. There is no significant or even tangible benefit to smaller source code. Fully compacting code also fully destroys readability. (minified js, anyone?) Less code is not always better. Better code is always better. Smaller code often is worse. (three letter names for every variable, anyone?) – MickLH Mar 15 '16 at 05:57
  • That sounds like a great use case for a simple hashset. Put all your options in the hashset, and check if `var` is in the hashset. Building a hashset is relatively expensive (not really important for 3 or 10 values of course), but since the options are known at compile-time, you can keep them global instead of recreating them. This also gives you the very useful ability to *name* the hashset, which improves readability. So you get a separate `AllowedFrobricatorClasses = hashset({"A", "B", "C"});` and a simple check `if (AllowedFrobricatorClasses.Contains(currentClass))`. – Luaan Mar 15 '16 at 09:27

9 Answers9

44

if you want to expand it compile time you can use something like this

template<class T1, class T2>
bool isin(T1&& t1, T2&& t2) {
   return t1 == t2;
}

template<class T1, class T2, class... Ts>
bool isin(T1&& t1 , T2&& t2, T2&&... ts) {
   return t1 == t2 || isin(t1, ts...);
}

std::string my_var = ...; // somewhere in the code
...
bool b = isin(my_var, "fun", "gun", "hun");

I did not test it actually, and the idea comes from Alexandrescu's 'Variadic templates are funadic' talk. So for the details (and proper implementation) watch that.

Edit: in c++17 they introduced a nice fold expression syntax

template<typename... Args>
bool all(Args... args) { return (... && args); }

bool b = all(true, true, true, false);
 // within all(), the unary left fold expands as
 //  return ((true && true) && true) && false;
 // b is false
Slava
  • 1,528
  • 1
  • 15
  • 23
  • this requires to list the options one by one. Is there a way to write something like `isin(my_var, {"fun", "gun", "hun"})` or `isin(my_vary, options_vector)` – Ruggero Turra Mar 14 '16 at 10:29
  • first option should be possible, but expanding the initializer_list compile time... not worth the effort really. For the second option I would rather use Kerrek answer, or maybe wrapped that into a more convenient interface like the one you asked for. Because that already sounds more like a runtime stuff. – Slava Mar 14 '16 at 10:41
  • the list of options is known at compile time – Ruggero Turra Mar 14 '16 at 10:42
  • 1
    @RuggeroTurra: the (untested) code actually answers your question, for checking whether there is an entry in a container, there is `std::find()` or more specialized functions (binary_search, lower_bound, count,...) – stefaanv Mar 14 '16 at 10:49
  • @stefaanv I kind of see the good use cases for what Ruggero requests. But unpacking initializer_list compile time is not trivial, I will try it later... and hopefully update. – Slava Mar 14 '16 at 10:59
  • @Slava: yet nothing? – Ruggero Turra Apr 13 '16 at 16:00
  • @Ruggero Look at Barry answer below. I think it does the thing, or at least goes the direction you want. I was thinking about compile time expansion first, but that does not make sense since the check is done run-time anyway. – Slava Apr 14 '16 at 08:13
27

The any_of algorithm could work reasonably well here:

#include <algorithm>
#include <initializer_list>

auto tokens = { "abc", "def", "ghi" };

bool b = std::any_of(tokens.begin(), tokens.end(),
                     [&var](const char * s) { return s == var; });

(You may wish to constrain the scope of tokens to the minimal required context.)

Or you create a wrapper template:

#include <algorithm>
#include <initializer_list>
#include <utility>

template <typename T, typename F>
bool any_of_c(const std::initializer_list<T> & il, F && f)
{
    return std::any_of(il.begin(), il.end(), std::forward<F>(f));
}

Usage:

bool b = any_of_c({"abc", "def", "ghi"},
                  [&var](const char * s) { return s == var; });
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • 9
    Be careful! Your statement `s == val` is comparing _pointers_! You probably want to use `std::strcmp` or one of its relatives. – Joshua Green Mar 14 '16 at 10:16
  • I might not be able to read C++ goo very well, but how does this make the goo know how to compare two `const char* s` with the == operator? Maybe avoid the goo and write a plain, bug free for loop... – Lundin Mar 14 '16 at 10:17
  • Note that the type of `var` was unclear in the original code. If it's also a `char *` (rather than, say, a `std::string`) then the OP would have similar issues. – Joshua Green Mar 14 '16 at 10:18
  • @Lundin, it is quite possible to compare pointers with `==`. However, that's probably not what's needed here. – Joshua Green Mar 14 '16 at 10:20
  • @JoshuaGreen I meant, how does it compare the contents rather than the addresses? – Lundin Mar 14 '16 at 10:22
  • 1
    @JoshuaGreen: `val` is an `std::string_view` of course :-) (Coming to think of it, the initializer list should be a list of string views, which would settle the problem once and for all.) Anyway, that's what the OP asked for. I assume he knows the semantics of his comparisons. – Kerrek SB Mar 14 '16 at 10:23
  • @Lundin, it _does_ compare the addresses. That's why I think it's wrong here. – Joshua Green Mar 14 '16 at 10:24
  • So the bug was created by the use of `auto`? Or does auto mean you get std::string rather than a `const char*`? Why use any of this crap in the first place, for something so trivial? It seems designed to break programs and create subtle bugs. I really need to add the C++ tag to my ignore list... – Lundin Mar 14 '16 at 10:26
  • @Ludin, actually, I was slightly confused by the ambiguous statement of the problem. In the above answer, the lambda takes `var` by reference and the tokens as `const char *`s. From the expression, we cannot determine what `var` is; we only know that it must be comparable to `const char *`s. If `var` is a `std::string` then `std::string`s will be constructed from the `const char *`s and everything will work as expected. If `var` is a `char *` then the `==` will simply compare the pointers. – Joshua Green Mar 14 '16 at 10:36
  • 1
    @Lundin: While the topic of strings and pointers is fascinating, I don't think it has anything to do with this question. The OP started with the exact same comparison syntax that I provide in the answer. The answer shows a model approach for the *structure* of the code. I trust that the OP can adapt the specifics of the comparisons to whatever actual types his real problem uses. All that is orthogonal to the question (and answer). – Kerrek SB Mar 14 '16 at 10:43
  • @KerrekSB, fair enough, though if someone searches for string comparison methods and finds this post, I'd hate for them to be bitten by that issue. – Joshua Green Mar 14 '16 at 10:45
  • But what have you gained from all of this obfuscation? Why can't you use a simple for loop? Give me one good reason. – Lundin Mar 14 '16 at 10:48
  • 5
    @Lundin: Raw code is a burden on the reader. It forces the reader to read your loop body. Giving a name to the concept and making a named call allows the reader to skip over the line "knowing what you mean", and trust the tested library implementation to have implemented the intention correctly. At a fundamental level, every *system* is limited by its growing complexity; named abstractions (algorithms) are one of the prime strategies for reducing complexity. – Kerrek SB Mar 14 '16 at 10:51
  • If the reader is so unbelievably incompetent that they can't even understand the simplest of `for` loops, then what makes you think they would understand all this meta programming nonsense any better? – Lundin Mar 14 '16 at 10:57
  • 3
    @Lundin: It's not about competence. It's about breaking down complexity. The reader has better things to do than look at your loop. – Kerrek SB Mar 14 '16 at 11:37
  • 1
    If you think the reader will interpret a simple, utterly fundamental, 2 seconds-to-understand, single-condition for loop _slower_ than the time it takes for them to interpret all of your meta programming, then further arguing with you is obviously pointless. – Lundin Mar 14 '16 at 11:59
  • FYI: your wrapper template can't compile like that - can't deduce from a braced-init-list (unfortunately). – Barry Mar 14 '16 at 17:44
  • 3
    @Lundin It's not so much "how hard is it to understand a `for` loop", rather "how easy is it to spot subtle errors_". E.g. off-by-1 errors at begin/end (easy if you have 1-based rather than more common 0-based array); and in more complex loop bodies there can be errors in terms of side-effects or error handling. Even though for loops are trivial, the _idea_ behind generic iteration is to eliminate certain common errors. Granted gains are minimal in trivial examples, and you wouldn't want to lose them by introducing other errors. But foreach, ifany, ifall, ifnone are simple, powerful concepts. – Disillusioned Mar 15 '16 at 00:27
  • 1
    @CraigYoung As you could notice from the above comments, the meta code could easily contain very subtle bugs, for example given that "var" was of type `const char*` and not some class which had an overloaded operator for that type. As for eliminating errors, they come from writing complex code where you could have written simple code. When you have a loop in its most simple form `for(i=0; i – Lundin Mar 15 '16 at 07:43
  • 1
    lots of accomplished programmers advocate the use of these small functions in their books, one of the main reasons its easy to (unit) test, others are that the code that uses it is more readable and doesnt break its level of abstraction, less code duplication, easier to extend/change. One of those sources I'm reading at the moment is: "Modern C++ Programming with Test-Driven Development" by Jeff Langr, he takes it very far and offers compelling reasons and examples to do so, definitely worth a read imho. Also check "clean code" by uncle bob. – Emile Vrijdags Mar 15 '16 at 19:35
12

Alrighty then, you want Radical Language Modification. Specifically, you want to create your own operator. Ready?

Syntax

I'm going to amend the syntax to use a C and C++-styled list:

if (x in {x0, ...}) ...

Additionally, we'll let our new in operator apply to any container for which begin() and end() are defined:

if (x in my_vector) ...

There is one caveat: it is not a true operator and so it must always be parenthesized as it's own expression:

bool ok = (x in my_array);

my_function( (x in some_sequence) );

The code

The first thing to be aware is that RLM often requires some macro and operator abuse. Fortunately, for a simple membership predicate, the abuse is actually not that bad.

#ifndef DUTHOMHAS_IN_OPERATOR_HPP
#define DUTHOMHAS_IN_OPERATOR_HPP

#include <algorithm>
#include <initializer_list>
#include <iterator>
#include <type_traits>
#include <vector>

//----------------------------------------------------------------------------
// The 'in' operator is magically defined to operate on any container you give it
#define in , in_container() =

//----------------------------------------------------------------------------
// The reverse-argument membership predicate is defined as the lowest-precedence 
// operator available. And conveniently, it will not likely collide with anything.
template <typename T, typename Container>
typename std::enable_if <!std::is_same <Container, T> ::value, bool> ::type
operator , ( const T& x, const Container& xs )
{
  using std::begin;
  using std::end;
  return std::find( begin(xs), end(xs), x ) != end(xs);
}

template <typename T, typename Container>
typename std::enable_if <std::is_same <Container, T> ::value, bool> ::type
operator , ( const T& x, const Container& y )
{
  return x == y;
}

//----------------------------------------------------------------------------
// This thunk is used to accept any type of container without need for 
// special syntax when used.
struct in_container
{
  template <typename Container>
  const Container& operator = ( const Container& container )
  {
    return container;
  }

  template <typename T>
  std::vector <T> operator = ( std::initializer_list <T> xs )
  {
    return std::vector <T> ( xs );
  }
};

#endif

Usage

Great! Now we can use it in all the ways you would expect an in operator to be useful. According to your particular interest, see example 3:

#include <iostream>
#include <set>
#include <string>
using namespace std;

void f( const string& s, const vector <string> & ss ) { cout << "nope\n\n"; }
void f( bool b ) { cout << "fooey!\n\n"; }

int main()
{
  cout << 
    "I understand three primes by digit or by name.\n"
    "Type \"q\" to \"quit\".\n\n";

  while (true)
  {
    string s;
    cout << "s? ";
    getline( cin, s );

    // Example 1: arrays 
    const char* quits[] = { "quit", "q" };
    if (s in quits) 
      break;

    // Example 2: vectors
    vector <string> digits { "2", "3", "5" };
    if (s in digits)
    {
      cout << "a prime digit\n\n";
      continue;
    }

    // Example 3: literals
    if (s in {"two", "three", "five"})
    {
      cout << "a prime name!\n\n";
      continue;
    }

    // Example 4: sets
    set <const char*> favorites{ "7", "seven" };
    if (s in favorites)
    {
      cout << "a favorite prime!\n\n";
      continue;
    }

    // Example 5: sets, part deux
    if (s in set <string> { "TWO", "THREE", "FIVE", "SEVEN" })
    {
      cout << "(ouch! don't shout!)\n\n";
      continue;
    }

    // Example 6: operator weirdness
    if (s[0] in string("014") + "689")
    {
      cout << "not prime\n\n";
      continue;
    }

    // Example 7: argument lists unaffected    
    f( s, digits );
  }
  cout << "bye\n";
}

Potential improvements

There are always things that can be done to improve the code for your specific purposes. You can add a ni (not-in) operator (Add a new thunk container type). You can wrap the thunk containers in a namespace (a good idea). You can specialize on things like std::set to use the .count() member function instead of the O(n) search. Etc.

Your other concerns

  • const vs mutable : not an issue; both are usable with the operator
  • laziness of or : Technically, or is not lazy, it is short-circuited. The std::find() algorithm also short-circuits in the same way.
  • compile time loop unrolling : not really applicable here. Your original code did not use loops; while std::find() does, any loop unrolling that may occur is up to the compiler.
  • easy to extend to operators other than == : That actually is a separate issue; you are no longer looking at a simple membership predicate, but are now considering a functional fold-filter. It is entirely possible to create an algorithm that does that, but the Standard Library provides the any_of() function, which does exactly that. (It's just not as pretty as our RLM 'in' operator. That said, any C++ programmer will understand it easily. Such answers have already been proffered here.)

Hope this helps.

Dúthomhas
  • 8,200
  • 2
  • 17
  • 39
9

First, I recommend using a for loop, which is both the easiest and most readable solution:

for (i = 0; i < n; i++) {
   if (var == eq[i]) {
      // if true
      break;
   }
}

However, some other methods also available, e.g., std::all_of, std::any_of, std::none_of (in #include <algorithm>).

Let us look at the simple example program which contains all the above keywords

#include <vector>
#include <numeric>
#include <algorithm>
#include <iterator>
#include <iostream>
#include <functional>

int main()
{
    std::vector<int> v(10, 2);
    std::partial_sum(v.cbegin(), v.cend(), v.begin());
    std::cout << "Among the numbers: ";
    std::copy(v.cbegin(), v.cend(), std::ostream_iterator<int>(std::cout, " "));
    std::cout << '\\n';

    if (std::all_of(v.cbegin(), v.cend(), [](int i){ return i % 2 == 0; })) 
    {
        std::cout << "All numbers are even\\n";
    }
    if (std::none_of(v.cbegin(), v.cend(), std::bind(std::modulus<int>(),
                                  std::placeholders::_1, 2))) 
    {
        std::cout << "None of them are odd\\n";
    }
    struct DivisibleBy
    {
        const int d;
        DivisibleBy(int n) : d(n) {}
        bool operator()(int n) const { return n % d == 0; }
    };

    if (std::any_of(v.cbegin(), v.cend(), DivisibleBy(7))) 
    {
        std::cout << "At least one number is divisible by 7\\n";
    }
}
Embedded C
  • 1,448
  • 3
  • 16
  • 29
6

You may use std::set to test if var belongs to it. (Compile with c++11 enabled)

#include <iostream>
#include <set>

int main()
{
    std::string el = "abc";

    if (std::set<std::string>({"abc", "def", "ghi"}).count(el))
        std::cout << "abc belongs to {\"abc\", \"def\", \"ghi\"}" << std::endl;

    return 0;
}

The advantage is that std::set<std::string>::count works in O(log(n)) time (where is n is number of strings to test) comparing to non compact if witch is O(n) in general. The disadvantage is that construction of the set takes O(n*log(n)). So, construct it once, like:

static std::set<std::string> the_set = {"abc", "def", "ghi"};

But, IMO it would be better to leave the condition as is, unless it contains more than 10 strings to check. The performance advantages of using std::set for such a test appears only for big n. Also, simple non compact if is easier to read for average c++ developer.

royhowie
  • 11,075
  • 14
  • 50
  • 67
ivaigult
  • 6,198
  • 5
  • 38
  • 66
5

The closest thing would be something like:

template <class K, class U, class = decltype(std::declval<K>() == std::declval<U>())>
bool in(K&& key, std::initializer_list<U> vals)
{
    return std::find(vals.begin(), vals.end(), key) != vals.end();
}

We need to take an argument of type initializer_list<U> so that we can pass in a braced-init-list like {a,b,c}. This copies the elements, but presumably we're going doing this because we're providing literals so probably not a big deal.

We can use that like so:

std::string var = "hi";    
bool b = in(var, {"abc", "def", "ghi", "hi"});
std::cout << b << std::endl; // true
Barry
  • 286,269
  • 29
  • 621
  • 977
4

If you have access to C++14 (not sure if this works with C++11) you could write something like this:

template <typename T, typename L = std::initializer_list<T>>
constexpr bool is_one_of(const T& value, const L& list)
{
    return std::any_of(std::begin(list), std::end(list), [&value](const T& element) { return element == value; });
};

A call would look like this:

std::string test_case = ...;
if (is_one_of<std::string>(test_case, { "first case", "second case", "third case" })) {...}

or like this

std::string test_case = ...;
std::vector<std::string> allowedCases{ "first case", "second case", "third case" };
if (is_one_of<std::string>(test_case, allowedCases)) {...}

If you don't like to "wrap" the allowed cases into a list type you can also write a little helper function like this:

template <typename T, typename...L>
constexpr bool is_one_of(const T& value, const T& first, const L&... next) //First is used to be distinct
{
    return is_one_of(value, std::initializer_list<T>{first, next...});
};

This will allow you to call it like this:

std::string test_case = ...;
if (is_one_of<std::string>(test_case, "first case", "second case", "third case" )) {...}

Complete example on Coliru

Simon Kraemer
  • 5,700
  • 1
  • 19
  • 49
2

Worth noting that in most Java and C++ code I've seen, listing 3 or so conditionals out is the accepted practice. It's certainly more readable than "clever" solutions. If this happens so often it's a major drag, that's a design smell anyway and a templated or polymorphic approach would probably help avoid this.

So my answer is the "null" operation. Just keep doing the more verbose thing, it's most accepted.

djechlin
  • 59,258
  • 35
  • 162
  • 290
  • I agree, but to refactor is out of my control. I just need a syntactic sugar. – Ruggero Turra Mar 14 '16 at 18:15
  • In my experience doing the most verbose thing quickly becomes so verbose its impossible to figure out what's actually being tested. With just 4 variables being tested and lego-naming, the test will easily fill 120+ characters on the screen. Adding 16 or 20 of such tests right after each other, and you haven't got a chance to figure out what's going on. – Clearer Jan 02 '18 at 20:38
  • @Clearer unit tests should generally be void of logic. For this reason they tend to be very verbose but very readable and error-free as standalone tests from each other. I'm not sure if you think 2400 characters is too many to be legible? My test files tend to be 1-2x as big as my source files which tend to run around 100s or 1000 lines. If you're having a major problem with about 10 tweets worth of characters being unreadable, then I'm not sure how you're writing shorter code any better. – djechlin Jan 08 '18 at 21:21
  • @djechlin I don't see how unit-tests cannot be made readable just because they are void of logic (which I disagree they generally should be). If your tests fail, I want to know which condition failed, not which group of conditions failed. – Clearer Jan 09 '18 at 13:10
  • @Clearer one test per condition. – djechlin Jan 10 '18 at 00:20
0

You could use a switch case. Instead of having a list of separate cases you could have :

include

using namespace std;

int main () { char grade = 'B';

switch(grade)
{
case 'A' :
case 'B' :
case 'C' :
    cout << "Well done" << endl;
    break;
case 'D' :
    cout << "You passed" << endl;
    break;
case 'F' :
    cout << "Better try again" << endl;
    break;

default :
    cout << "Invalid grade" << endl;

}

cout << "Your grade is " << grade << endl;

return 0;

}

So you can group your results together: A, B and C will output "well done". I took this example from Tutorials Point: http://www.tutorialspoint.com/cplusplus/cpp_switch_statement.htm

Mike Cave
  • 147
  • 1
  • 4
  • 18
  • definitevely not better than a list of `if`s – Ruggero Turra Apr 05 '16 at 11:46
  • Apparently that's debatable: [link] (http://stackoverflow.com/questions/97987/advantage-of-switch-over-if-else-statement) and here: [lnk](http://stackoverflow.com/questions/6805026/is-switch-faster-than-if) – Mike Cave Apr 13 '16 at 09:40
  • I asked for a more *compact* version, not faster – Ruggero Turra Apr 13 '16 at 09:52
  • Um.. you asked for >"Is there a better way to write code like this: " ? Which I suppose was quite an open question. Apologies if I misunderstood you :-) – Mike Cave Apr 13 '16 at 10:02