适合多行 if 条件的良好 C 编码风格

问题描述 投票：0回答：7

我正在用 C 语言编写一个项目，遇到一个问题：我有很多 

if
条件，其他人可能很难阅读。我还没在网上找到类似的问题。

您有如何使我的代码更具可读性的想法或示例吗？

这是 C 代码：

if( ((g_cycle_cnt == uartTxSecondaryMsg[3][msgPos[3]].sliceNo) ||   //correct slicenumber...
    (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -1) ||             // or as fast as possible...                                            

  ( (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -2) &&
   ((uartTxSecondaryMsg[3][msgPos[3]].timeFrameBegin>=g_uptime_cnt) && 
    (uartTxSecondaryMsg[3][msgPos[3]].timeFrameEnd<=g_uptime_cnt)))) &&

   ((dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) == SECONDARY_MSG_ANNOUNCED_CH4) )
c if-statement conditional-statements lines
7个回答
12
投票

创建具有指示性名称的函数，用于检查需求并代表其含义，例如：

if( is_correct_slice_number(/*... params here ... */) || 
    is_asap(/*... params here ... */)  || 
    is_other_condition(/*... params here ... */))

或者作为遵循相同逻辑的建议宏，例如：

if( IS_CORRECT_SLICE_NUMBER(/*... params here ... */) || 
    IS_ASAP(/*... params here ... */)  || 
    IS_OTHER_CONDITION(/*... params here ... */))

我认为这可能会让你的意图更清楚。

7
投票

如果您想坚持使用现有代码（而不是将内容分解为内联函数），并且只修改缩进，我非常喜欢始终使用 

indent
。这意味着您可以修复任何源文件。

通过其默认选项，它为您提供 GNU 缩进，即：

if (((g_cycle_cnt == uartTxSecondaryMsg[3][msgPos[3]].sliceNo) ||       //correct slicenumber...
     (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -1) ||        // or as fast as possible...
     ((uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -2) &&
      ((uartTxSecondaryMsg[3][msgPos[3]].timeFrameBegin >= g_uptime_cnt) &&
       (uartTxSecondaryMsg[3][msgPos[3]].timeFrameEnd <= g_uptime_cnt)))) &&
    ((dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) ==
     SECONDARY_MSG_ANNOUNCED_CH4))
  {
    /* do something */
  }

我想说，这里的问题实际上是您在数组中难以辨认地闲逛。至少，要排除：

uartTxSecondaryMsg[3][msgPos[3]]

到一个单独的变量中，例如：

whatever *foo = &uartTxSecondaryMsg[3][msgPos[3]];
if (((g_cycle_cnt == foo->sliceNo) ||   //correct slicenumber...
     (foo->sliceNo == -1) ||    // or as fast as possible...
     ((foo->sliceNo == -2) &&
      ((foo->timeFrameBegin >= g_uptime_cnt) &&
       (foo->timeFrameEnd <= g_uptime_cnt)))) &&
    ((dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) ==
     SECONDARY_MSG_ANNOUNCED_CH4))
  {
    /* do something */
  }

显然要为

foo
选择合适的类型和变量名称。

然后，您可以将 

if
语句的各个部分分离成单独的函数，每个函数将 
foo
作为参数。

3
投票

[这个很主观]

  • 删除多余的括号；他们是防御性的并且模糊了含义
  • 正确对齐和排序条件（可能忽略缩进规则）
  • （也许）重写为另一个构造，例如 
    switch
    ，或使用早期的 
    return
    ，甚至 
    goto

第一步（清理和对齐）：

if (    (  uartTxSecondaryMsg[3][msgPos[3]].sliceNo == g_cycle_cnt //correct slicenumber...
        || uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -1          // or as fast as possible...                                
        ||(uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -2
             && uartTxSecondaryMsg[3][msgPos[3]].timeFrameBegin >= g_uptime_cnt
             && uartTxSecondaryMsg[3][msgPos[3]].timeFrameEnd <= g_uptime_cnt
             && (dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) == SECONDARY_MSG_ANNOUNCED_CH4 ) ) 
                {
                // do something useful here
                }

第二步，使用

switch
（和
goto
！）[这对某些读者来说可能有点太多了......]

switch (uartTxSecondaryMsg[3][msgPos[3]].sliceNo) {
default:
    if (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == g_cycle_cnt) goto process;
    break;
case -2:
    if (uartTxSecondaryMsg[3][msgPos[3]].timeFrameBegin < g_uptime_cnt) break;
    if (uartTxSecondaryMsg[3][msgPos[3]].timeFrameEnd > g_uptime_cnt) break;
    if ((dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) != SECONDARY_MSG_ANNOUNCED_CH4) break;
case -1:
process:

          // do something useful here
 }


switch()
构造的优点是可以立即清楚地知道
uartTxSecondaryMsg[3][msgPos[3]].sliceNo
是master条件。另一个优点是可以添加案例和条件，而不必干扰其他案例或与括号作斗争。

现在我希望我能正确删除括号......

2
投票

这是一个很好的例子，说明了缩进为何如此重要。当我阅读其他人的解决方案时，我意识到有些人弄错了最后一个 && 子句。当我使用 Notepad++ 突出显示括号时，我发现最后一个 && 子句实际上处于最高级别，而大多数重写都将其与“....timeFrameEnd<=g_uptime_cnt"

”配对

另外，看第三个子句，请注意 (test_1 && (test_2 && test_3)) 相当于 (test_1 && test_2 && test_3)，因此只需删除一层括号即可消除一些复杂性。

我更喜欢示例 C。

// Example A:
if (  (  (g_cycle_cnt == uartTxSecondaryMsg[3][msgPos[3]].sliceNo)  //correct slicenumber
      || (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -1)           // or as fast as possible...
      || (  (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -2)
         && (  (uartTxSecondaryMsg[3][msgPos[3]].timeFrameBegin>=g_uptime_cnt)
            && (uartTxSecondaryMsg[3][msgPos[3]].timeFrameEnd<=g_uptime_cnt))))
      && ((dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) == SECONDARY_MSG_ANNOUNCED_CH4)
   ){
    // do something
}

// Example B
if ( ( (g_cycle_cnt == uartTxSecondaryMsg[3][msgPos[3]].sliceNo) ||   //correct slicenumber...
       (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -1) ||             // or as fast as possible...
       ( (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -2) &&
         ( (uartTxSecondaryMsg[3][msgPos[3]].timeFrameBegin >= g_uptime_cnt) &&
           (uartTxSecondaryMsg[3][msgPos[3]].timeFrameEnd <= g_uptime_cnt)))) &&

     ( (dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) == SECONDARY_MSG_ANNOUNCED_CH4) 
   )
{
    // do something
}

// Example C
if( ( (g_cycle_cnt == uartTxSecondaryMsg[3][msgPos[3]].sliceNo) ||   //correct slicenumber...
      (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -1) ||             // or as fast as possible...
      ( (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -2) &&
        (uartTxSecondaryMsg[3][msgPos[3]].timeFrameBegin>=g_uptime_cnt) && 
        (uartTxSecondaryMsg[3][msgPos[3]].timeFrameEnd<=g_uptime_cnt)
      )
    ) &&
    ( (dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) == SECONDARY_MSG_ANNOUNCED_CH4) 
  )
{
    // do something
}
1
投票

这确实是主观的（即，关于如何使这类事情变得更好的意见几乎与有多少人一样多）。

我可能会使用一些额外的变量，例如

WhateverType  your_object = uartTxSecondaryMsg[3][msgPos[3]];
int slice = your_object.sliceNo;
int begin = your_object.timeFrameBegin;
int end = your_object.timeFrameEnd;

if ( ( g_cycle_cnt == slice || 
       slice == -1 ||
       (slice == -2 && begin >= g_uptime_cnt && end <= g_uptime_cnt)
     ) &&
     ((dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) == SECONDARY_MSG_ANNOUNCED_CH4) 
   )
0
投票

通过功能设计你可以做这样的事情。如果您愿意，也可以使用 else if 结构，以便代码更紧凑，但我更喜欢这个。这样水平代码就最少了。也许我看太多Linux内核代码了。但我认为这是内核空间中的人可以做到的方式。

你的“如果”是如此怪物以至于没有人能够遵循它。这种风格你可以逐行检查，如果需要的话还可以在 if 语句之前添加注释。

void foo()
{
    if (dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4 != SECONDARY_MSG_ANNOUNCED_CH4)
        return;

    if (g_cycle_cnt == uartTxSecondaryMsg[3][msgPos[3]].sliceNo)
        goto out;

    if (uartTxSecondaryMsg[3][msgPos[3]].sliceNo != -2)
        return;

    if (uartTxSecondaryMsg[3][msgPos[3]].timeFrameBegin < g_uptime_cnt)
        return;

    if (uartTxSecondaryMsg[3][msgPos[3]].timeFrameEnd > g_uptime_cnt)
        return;
out:
    // Do something.

    // Another way is to call another function here and also in goto statement
    // That way you don't have to use goto's if do not want.
}
0
投票

我假设您通常已将括号和换行符放在您想要的位置，而不是搞乱它们。 我认为将解析树建模为使用 OTBS 的缩进树，但无论是单行给定节点还是多行，都是开发人员的偏好，但要理解的是，您要么对每个子节点进行多行处理，要么对所有子节点进行单行处理。 我的规则：

如果值得对表达式进行多行处理，则它会得到 OTBS 处理。在第一行末尾打开，在块父对象末尾缩进关闭。
  1. 中缀运算符位于行首。
  2. 单行可以跳多级，只需缩进一层，但进入的层数必须与行尾出去的层数一致。
    4. 逻辑：

对于解析树中的对等行，行与其上方行的关系是
    始终
  1. 位于该行的开头。 对于父母/孩子，第一个孩子与其父母之间的关系位于父母行的末尾。
  2. 对于结局来说，它们总是独立的。
  3. 缩进部分的结尾始终与该部分的开头处于相同的缩进级别。
    5. if( ( //correct slicenumber... (g_cycle_cnt == uartTxSecondaryMsg[3][msgPos[3]].sliceNo) // or as fast as possible... || (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -1) || ( (uartTxSecondaryMsg[3][msgPos[3]].sliceNo == -2) && ( (uartTxSecondaryMsg[3][msgPos[3]].timeFrameBegin>=g_uptime_cnt) && (uartTxSecondaryMsg[3][msgPos[3]].timeFrameEnd<=g_uptime_cnt) ) ) ) && ((dataProcessingFlag & SECONDARY_MSG_ANNOUNCED_CH4) == SECONDARY_MSG_ANNOUNCED_CH4) ) { //do something } 
现在，如果这是我的代码，我还会考虑减少一些多余的括号，从而使树稍微扁平化，但是为了缩进？  是的，那种“OTBS，但用于括号”和“将中缀运算符放在开头”是我的首选风格。

最新问题
© www.soinside.com 2019 - 2024. All rights reserved.