有没有一种方法可以缩短这个while条件？

Question

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
}

为了更清晰： temp 是一个指向以下node结构的指针：

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

- Lukáš Pištěk

---

你不能在所有等号运算符内部进行优化，而不了解temp->left和temp->right之间的依赖关系。从视觉上看，你可以使用正则表达式，但在内部实现上可能是相当或甚至更加低效的。 - U. Windl

我很想知道你为什么认为自己有这个问题。这似乎是对表达式树进行运行时解释，如果是这样的话，有更好的方法可以解决它。 - user207421

Answer 1

当然，你可以使用一系列有效的运算符并搜索它。

#include <cstring>

// : :

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

如果您担心性能问题，那么也许可以考虑使用表格查找：

#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
}

- paddy

---

对于strchr函数需要小心使用，因为这个函数也会在temp->left->oper或者temp->right->oper等于'\0'时返回真。不过实际上这可能是一个不错的解决方案。 - john

你可能希望将实现细节提取到一个单独的函数中。 - L. F.

在这种情况下，为什么要使用 strchr 而不是 std::string 的 .find() 函数？ - scohe001

是的，楼主的目标是缩短循环条件。使用 ops.find(temp->left->oper) != std::string::npos 比使用 strchr 更短。但当然，正如评论中指出的那样，对于搜索 \0 的情况，strchr 的行为是不同的，因此使用它也可能被认为是潜在错误或实际错误，这取决于输入。 - paddy

@paddy，我认为他不是在寻找一个压缩过的答案！只是想要避免冗长的||语句链。 - Baldrickk

Answer 2

是的，确实可以！

将有效字符存储到一个std::array 或者一个普通数组中，并使用标准算法std::any_of来检查条件。

#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
}

---

将此内容封装到一个函数中，该函数接受要检查的node对象，可以使其更加简洁。

#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);
}

可以在while循环中调用

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

- JeJo

---

应该使用std::any_of而不是std::all_of吗？ - Vishaal Shankar

@displayName tester 也可以是 const，并且 const 的韵律可以被保持。 - JeJo

@JeJo: 想这样做，但我不确定。作为一个C++的新手。 - displayName

当字符串字面量可以胜任时，不需要使用数组，当std::string（或std::string_view）具有.find()或.find_first_of()时，不需要使用any_of。// isValid也应该是一个lambda表达式，这样你就可以将它保持在while循环的正上方，而不是离使用的地方太远。https://dev59.com/k1MI5IYBdhLWcg3wDnKG#63210165 // 另外，const node＆temp使用.访问，而不是->访问。你可能想传递const node* temp。 - user10063119

@ankii，当然还有其他方法。你的sln看起来更加紧凑和有前途。除非isValid只在整个代码库中使用一次并且几乎不是内联的，否则它不必成为一个lambda。顺便说一句，感谢指出拼写错误。我已经纠正了它。 - JeJo

Answer 3

创建一个子函数。

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

然后：

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

- Jarod42

---

作为说明，这样的函数也可以添加到“node”，或者作为继承自“node”的类的一部分，而不会添加任何新的数据成员，也不会破坏其标准布局。 - Justin Time - Reinstate Monica

我从来不被那种 "微重构" 所说服。它所做的就是将代码中任何错误从你可以在上下文中看到它们的位置移动到你无法看到的某个地方。当然，如果相同的测试在应用程序的其他位置出现，那么将其分解是一个很好的理由。 - alephzero

@alephzero 在这种情况下，我更喜欢使用这种类型的子函数，因为它们具有以下优点：（1）如果您想要添加运算符，则可以抽象出可能会发生变化的定义；（2）它们提供了一个小型可测试的接口；（3）它们可以在头文件中进行文档化。 - WorldSEnder

如果您还没有看过@alephzero的演讲《测试性和良好设计之间的深层协同》，建议您观看一下。它可能会给您带来另一个视角。 - displayName

@alephzero：如果它只在一个地方使用（两次），而且您不想公开它，您仍然可以创建lambda。 - Jarod42

@alephzero 一个使用lambda表达式的答案 https://dev59.com/k1MI5IYBdhLWcg3wDnKG#63210165 - user10063119

Answer 4

C语言风格：

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

如果您要声明变量，可能需要使用花括号将// Do something括起来。

- MCCCS

---

这不仅仅是“C风格”。在这种情况下，有非常好的理由选择这样做，因为编译器能够比一堆（理论上）没有关联的if语句更有效地构建跳转表。即使无法构建跳转表，“我正在将一大堆不同的常量与同一个变量进行比较”也是向编译器提供的很好的信息，可以帮助它更好地进行优化。 - T.E.D.

最好将条件更改为 while (1)；将第一个 break 更改为 continue；将 default 情况更改为 break；在 switch 后添加另一个 break。我在解释器中多次使用了这种模式。 - user207421

@T.E.D. 我对你的前提持怀疑态度。一堆级联的 switch case 和由 OR 连接的 if 语句并没有什么不同。人类可能在视觉上无法识别，但我会感到惊讶如果编译器不支持这两种方式。 - Alexander

@Alexander 不是的，GCC 9.1增加了跳转表、位测试和决策树来处理switch语句。TCase1 TCase2 - MCCCS

@MCCCS 很有趣，感谢您提供这个。看到这个让我很烦恼。在我看来，switch和if/else if/else是同构的（特殊情况下，所有谓词都针对单个“切换”值进行检查），而空的级联案例体与谓词上的OR条件相同。为什么编译器看不到呢 :| - Alexander

Answer 5

你可以构造一个包含选项的字符串，并搜索字符：

#include <string>

// ...

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

< p >< code > "+-*/"s 是 C++14 的一个特性。在 C++14 之前，请使用 < code > std::string ops = "+-*/"; 。

- L. F.

Answer 6

编程是找到冗余并消除它们的过程。

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
}

这里的“重复单元”是什么？嗯，我看到两个实例。

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

所以让我们将重复的部分提取出来成为一个函数，这样我们只需要写一次就可以了。

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
}

哒哒哒！缩短了！
(原始代码：17行，其中8行是循环条件。修订后的代码：18行，其中2行是循环条件。)

- Quuxplusone

---

我对现代C++不是很熟悉，但是难道没有一种简单的方式来表达类似于['+', '-', '*', '/'].contains(this->oper)这样的语句吗？ - Alexander

@Alexander：不，如果你真的想进一步缩短oper_is_arithmetic()函数，你可以写成return "+-*/"s.find(this->oper) != std::string::npos;，但是这种 Perl 风格的代码比使用普通的this->oper == '+' || ...更难读懂。 - Quuxplusone

啊，iOS世界中有一些API使用这种模式。与其在String上拥有像func contains(substring: String) -> Bool这样的方法，不如使用func range(of: String) -> NSRange，它返回包含匹配子字符串的索引范围，如果没有找到则返回一个标志值（NSNotFound，类似于std::string::npos）。我非常讨厌这种模式，难道添加一个额外的函数来返回一个简单的布尔值真的很难吗？ - Alexander

Answer 7

"

" + "-" + "+" + "*" + "/" + "

是ASCII十进制值为42、43、45和47，因此"

#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 */ }

- user3281036

---

我个人建议将其制作为帮助函数而非宏定义，并且最好记录下其功能，以防未来的维护者不熟悉ASCII码点。 - Justin Time - Reinstate Monica

你可以直接使用 '+' 来表示符号，无需记忆任何ASCII代码。 - HolyBlackCat

我认为在这种特定情况下使用ASCII码可能更好，@HolyBlackCat，因为使用范围测试而不是直接比较；x> 41比x> ')'更易读，而x <'0'则一定会让至少一些人感到惊讶。 （尽管在这种情况下，应该注意ASCII代码点的使用，并且它只与ASCII系统兼容。） - Justin Time - Reinstate Monica

说实话，我认为这个想法还不错，因为这是一种微观优化类型，我不认为编译器经常会做。但是，如果关注性能优化，最好从x < 48开始，以最大化从最左边的比较中短路运算符的数量。 - Justin Time - Reinstate Monica

我认为这是一个相当不错的想法，即使实现有点瑕疵。 ASCII表顺序被设计用来促进这种使用方式。如果通常的情况下条件都是false，那么这可能会表现得更好，因为大多数字母都会在第二次检查时失败。如果OP决定支持括号，那么这甚至可以使用位掩码来完成。 - sudo rm -rf slash

Answer 8

正则表达式来拯救！

#include <regex>

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

解释：正则表达式中的方括号 [] 表示一个正则字符类。这意味着“匹配方括号内列举的任何一个字符”。例如，g[eiou]t 可以匹配 "get"、"git"、"got" 和 "gut"，但不匹配 "gat"。在字符类内部，加号（+）、减号（-）、星号（*）和斜杠（/）需要使用反斜杠进行转义。

免责声明：我没有时间运行这段代码；你可能需要进行微调，但是你已经有了思路。你可能需要将 oper 从 char 类型转换为 std::string 类型。

参考资料
1. http://www.cplusplus.com/reference/regex/regex_match/
2. https://www.rexegg.com/regex-quickstart.html
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

- kmiklas

---

这样做效率相当低下，因为每次循环迭代都要构造 std::regex 对象，而且正则表达式需要不断地重新编译。将其提取为常量已经可以帮助提高效率了。 - TheOperator

Answer 9

在时间和空间之间做出取舍，您可以构建两个“布尔”数组，分别由temp->left->oper和temp->left->oper索引。 相应的数组包含true（当条件满足时）和false（否则）。

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

由于左右的集合看起来相同，因此只需要一个数组。

初始化应该像这样：

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

...

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

对于array2同样适用。由于现代流水线CPU不善于跳转，因此您甚至可以使用这样一个更大的表：

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

但是初始化更加棘手：

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('/');

- U. Windl

---

您还需要将 char 索引强制转换为 unsigned char，以避免在 char 为有符号且为负数时可能发生越界访问。 - Jarod42

@Jarod42：我已经很久没有编写C++代码了，所以这段代码是我凭记忆写的。如果有需要修改的地方，请随意提出建议（例如：编辑！）。 - U. Windl

Answer 10

将运算符放入unordered_set中将更加高效，并且提供O(1)的访问速度。"Original Answer"翻译成"最初的回答"。

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                                                                                                                
}

- Spartan

---

在这种情况下，O(1) 的速度将比 O(n) 慢得多。 - L. F.

你能否请稍微解释一下。 - Spartan

O-符号是一种渐近符号，意味着常数被“吸收”。在这种情况下，unordered_set的开销将显著支配线性搜索4个字符的成本。 - L. F.