Is there a way to shorten this while condition?

C++AlgorithmIf StatementWhile LoopSimplify

C++ Problem Overview


while (temp->left->oper == '+' || 
       temp->left->oper == '-' || 
       temp->left->oper == '*' || 
       temp->left->oper == '/' || 
       temp->right->oper == '+' || 
       temp->right->oper == '-' || 
       temp->right->oper == '*' || 
       temp->right->oper == '/')
{
    // do something
}

For clarity: temp is a pointer that points to following node structure:

struct node
{
    int num;
    char oper;
    node* left;
    node* right;
};

C++ Solutions


Solution 1 - C++

Sure, you could just use a string of valid operators and search it.

#include <cstring>

// : :

const char* ops = "+-*/";
while(strchr(ops, temp->left->oper) || strchr(ops, temp->right->oper))
{
     // do something
}

If you are concerned about performance, then maybe table lookups:

#include <climits>

// : :

// Start with a table initialized to all zeroes.
char is_op[1 << CHAR_BIT] = {0};

// Build the table any way you please.  This way using a string is handy.
const char* ops = "+-*/";
for (const char* op = ops; *op; op++) is_op[*op] = 1;

// Then tests require no searching
while(is_op[temp->left->oper] || is_op[temp->right->oper])
{
     // do something
}

Solution 2 - C++

Yes, indeed you can!

Store the valid-characters to a std::array or even a plain array and apply the standard algorithm std::any_of to it for checking the condition.

#include <array>     // std::array
#include <algorithm> // std::any_of

static constexpr std::array<char, 4> options{ '+', '-', '*', '/' };
const auto tester = [temp](const char c) { return temp->left->oper == c || temp->right->oper == c; };
const bool isValid = std::any_of(options.cbegin(), options.cend(), tester);

while(isValid) // now the while-loop is simplified to
{
	// do something
}

This can be more cleaned by packing into a function, which accepts the node object to be checked.

#include <array>     // std::array
#include <algorithm> // std::any_of

bool isValid(const node *const temp) /* noexcept */
{
   static constexpr std::array<char, 4> options{ '+', '-', '*', '/' };
   const auto tester = [temp](const char c) { return temp->left->oper == c || temp->right->oper == c; };
   return std::any_of(options.cbegin(), options.cend(), tester);
}

which can be called in the while-loop

while (isValid(temp)) // pass the `node*` to be checked
{
	// do something
}

Solution 3 - C++

Create a sub function,

bool is_arithmetic_char(char)
{
// Your implementation or one proposed in another answers.
}

and then:

while (is_arithmetic_char(temp->left->oper)
    || is_arithmetic_char(temp->right->oper))
{
    // do something
}

Solution 4 - C++

C-style:

int cont = 1;
while(cont)
    switch(temp->left->oper) {
    case '+':
    case '-':
    ...
    case '/':
        // Do something
        break;
    default:
        cont = 0;
    }

You might need to enclose // Do something with curly braces if you're going to declare variables.

Solution 5 - C++

You can construct a string that contains the options and search for the character:

#include <string>

// ...

for (auto ops = "+-*/"s; ops.find(temp-> left->oper) != std::string::npos ||
                         ops.find(temp->right->oper) != std::string::npos;)
    /* ... */;

The "+-*/"s is a C++14 feature. Use std::string ops = "+-*/"; before C++14.

Solution 6 - C++

Programming is the process of finding redundancies and eliminating them.

struct node {
    int num;
    char oper;
    node* left;
    node* right;
};

while (temp->left->oper == '+' || 
       temp->left->oper == '-' || 
       temp->left->oper == '*' || 
       temp->left->oper == '/' || 
       temp->right->oper == '+' || 
       temp->right->oper == '-' || 
       temp->right->oper == '*' || 
       temp->right->oper == '/') {
    // do something
}

What's the "repeated unit" here? Well, I see two instances of

   (something)->oper == '+' || 
   (something)->oper == '-' || 
   (something)->oper == '*' || 
   (something)->oper == '/'

So let's factor that repeated part out into a function, so that we only have to write it once.

struct node {
    int num;
    char oper;
    node* left;
    node* right;

    bool oper_is_arithmetic() const {
        return this->oper == '+' || 
               this->oper == '-' || 
               this->oper == '*' || 
               this->oper == '/';
    }
};

while (temp->left->oper_is_arithmetic() ||
       temp->right->oper_is_arithmetic()) {
    // do something
}

Ta-da! Shortened!
(Original code: 17 lines, 8 of which are the loop condition. Revised code: 18 lines, 2 of which are the loop condition.)

Solution 7 - C++

"+" "-" "*" and "/" are ASCII decimal values 42, 43, 45 and 47 thus

#define IS_OPER(x) (x > 41 && x < 48 && x != 44 && x != 46)

while(IS_OPER(temp->left->oper || IS_OPER(temp->right->oper){ /* do something */ }

Solution 8 - C++

Regex to the rescue!

#include <regex>

while (
    std::regex_match(temp->left->oper, std::regex("[\+\-\*\/]")) ||
    std::regex_match(temp->right->oper, std::regex("[\+\-\*\/]"))
) { 
// do something
}

EXPLANATION: Regex brackets [] denote a regex "character class." This means "match any character listed inside the brackets." For example, g[eiou]t would match "get," "git," "got," and "gut," but NOT "gat." The backslashes are needed because plus (+) minus (-) and star (*) and forward-slash (/) have meaning within a character class.

DISCLAIMER: I don't have time to run this code; you might have to tweak it, but you get the idea. You might have to declare/convert oper from a char to a std::string.

REFERENCES

  1. http://www.cplusplus.com/reference/regex/regex_match/<br />
  2. https://www.rexegg.com/regex-quickstart.html<br />
  3. https://www.amazon.com/Mastering-Regular-Expressions-Jeffrey-Friedl/dp/0596528124/ref=sr_1_1?keywords=regex&qid=1563904113&s=gateway&sr=8-1

Solution 9 - C++

Trading space against time, you could build two "Boolean" arrays indexed by temp->left->oper and temp->left->oper, respectively. The corresponding array contains true when the condition is met, false otherwise. So:

while (array1[temp->left->oper] || array1[temp->right->oper]) {
// do something
}

As the sets for left and right seem identical, one array will actually do.

Initialization would be like this:

static char array1[256]; // initialized to "all false"

...

array1['+'] = array1['-'] = array1['*'] = array1['/'] = '\001';

Similar for array2. As jumps are bad for modern pipelining CPUs, you could even use a larger table like this:

while (array1[temp->left->oper << 8 | temp->right->oper]) {
    // do something
}

But initialization is more tricky:

static char array1[256 * 256]; // initialized to "all false"

...

void init(char c) {
    for (unsigned char i = 0; i <= 255; ++i) {
        array1[(c << 8) | i] = array1[(i << 8) | c] = '\001';
    }
}

init('+');
init('-');
init('*');
init('/');

Solution 10 - C++

Putting the operators in the unordered_set will be lot efficient and will provide O(1) access to operators.

unordered_set<char> u_set;                                                                                                                                                   
u_set.insert('+');                                                                                                                                                           
u_set.insert('*');                                                                                                                                                           
u_set.insert('/');                                                                                                                                                           
u_set.insert('-');                                                                                                                                                           
                                                                                                                                                          

if((u_set.find(temp->left->oper) != u_set.end()) || (u_set.find(temp->right->oper) != u_set.end())) {     
                 //do something                                                                                                                
}

Solution 11 - C++

Lambda & std::string_view

string_view gives lots of functionality of std::string and can operator on literals and it doesn't own the string.

Use Lambda instead of function for a highly local code which is not of use for the rest of the file. Also, no need to pass variables when lambda can capture them. Also get inline benefits without specifying it for a function you'd otherwise create.

Make char const:

auto is_arithm = [](const char c) {
  return std::string_view("+-/*").find_first_of(c) != std::string::npos;
};

while (is_arithm(temp->left->oper) || is_arithm(temp->right->oper)) {
}


You can change const char c to const node *t access its oper member inside the lambda. But that is not a good idea since members of left/right of temp can be modified.

auto is_arithm2 = [](const node *t) {
  return std::string_view("+-/*").find_first_of(t->oper) != std::string::npos;
};

while(is_arithm2(temp->left) || is_arithm2(temp->right)){
}

Attributions

All content for this solution is sourced from the original question on Stackoverflow.

The content on this page is licensed under the Attribution-ShareAlike 4.0 International (CC BY-SA 4.0) license.

Content TypeOriginal AuthorOriginal Content on Stackoverflow
QuestionLuk&#225;š PištěkView Question on Stackoverflow
Solution 1 - C++paddyView Answer on Stackoverflow
Solution 2 - C++JeJoView Answer on Stackoverflow
Solution 3 - C++Jarod42View Answer on Stackoverflow
Solution 4 - C++MCCCSView Answer on Stackoverflow
Solution 5 - C++L. F.View Answer on Stackoverflow
Solution 6 - C++QuuxplusoneView Answer on Stackoverflow
Solution 7 - C++user3281036View Answer on Stackoverflow
Solution 8 - C++kmiklasView Answer on Stackoverflow
Solution 9 - C++U. WindlView Answer on Stackoverflow
Solution 10 - C++SpartanView Answer on Stackoverflow
Solution 11 - C++user10063119View Answer on Stackoverflow