如果有条件,多行的良好 C 编码风格

Good C-coding style for multiple lines if conditions

我正在用 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) )

创建具有指示性名称的函数,用于检查要求并表示其含义,例如:

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 ... */))

我认为这可能会使您的意图更加明确。

如果您想坚持使用现有代码(而不是将内容分解到内联函数中),并且只修改缩进,我非常喜欢始终如一地使用 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 作为参数。

这真的很主观(即关于如何使这类事情变得更好的意见几乎与人一样多)。

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

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) 
   )

[这个很主观]

  • 删除过多的括号;他们是防御性的并且模糊了意义
  • 正确对齐和排序条件(可能忽略缩进规则)
  • (也许)重写成另一个构造,例如 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]].sliceNomaster 条件。另一个优点是可以添加案例和条件,而不必干扰其他案例或为括号而烦恼。

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

这是一个很好的例子,说明为什么缩进如此重要。当我阅读其他人的解决方案时,我意识到有些人弄错了最后一个 && 子句。当我使用 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
}

通过功能设计,你可以做这样的事情。如果你愿意,你也可以使用 else if 结构,这样代码更紧凑,但我更喜欢这个。这样横向代码最少。也许我看了太多 Linux 内核代码。但我认为这是内核 space 中某人可以做到这一点的方式。

你的 if 太怪异了,没人能跟得上它。您可以逐行检查这种样式,如果需要,还可以在 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.
}