Is there a way to shorten this while condition?
C++AlgorithmIf StatementWhile LoopSimplifyC++ 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
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++
std::string_view
Lambda & 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.
-
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rf-capture-vs-overload
-
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rstr-view
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)){
}