logo标签列表

多行if条件语句的良好C编码风格

cif-statementconditional-statementslines
8

我正在使用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) )
1
你有什么想法或示例可以使它更易读吗？使用适当的缩进？ - alk
当然。OP正在询问如何做到这一点。 - abligh
这似乎是从Eclipse复制到此窗口的问题...我在发布后才意识到错误。 - En Kt
@abligh：啊好吧...好的，因为你想学习如何清理，所以加1分！;-) - alk
1
@EnKt: “我发现错误已经发布了。”那么修复它怎么样？ - alk
6个回答
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 ... */))

我认为这可能会使你的意图更清晰。
1+，不过宏也可以。 - alk
@alk 感谢您的建议 :) - Scis
实际上，这3个检查的集合本身也必须具有某种含义，并且可以封装在一个更具描述性的名称中。 - Philip Stuyck
谢谢！我会考虑它是否适用于这种情况。 - En Kt
6
如果你想继续使用现有的代码（而不是将其拆分为内联函数），只需修改缩进，我非常喜欢始终使用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作为参数。

那似乎是个好主意！非常感谢！我会立刻使用你的指针想法。 :) - En Kt
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是主条件。另一个优点是可以添加案例和条件，而无需干扰其他案例或与括号斗争。

现在我希望我已经正确删除了括号...

我的同事们会因为我使用goto而杀了我...但是感谢你向我展示了一个良好对齐的例子。 - En Kt
我明白，无论是对齐还是跳转，都可能违反编码样式指南。但对于有些人来说，可读性更为重要。 - wildplasser
缩进并将逻辑运算符放在行首是非常好的方式，可以清晰地展示条件的顺序（我自己也经常使用这种方式。我赞同这个答案）。switch语句会使代码变得晦涩难懂，无法给我提供概览（我应该再次对这个答案进行点踩）。然而，'{'和'}'应该回到正常的缩进级别，即从'if'列开始缩进一个制表符，因为当第三个OR为真时，动作并不会开始（但任何OR为真时都会开始）。 - Paul Ogilvie
我本来可能会搞砸，去掉了 {}。在这种情况下，switch 有点过头了，但在像这样的协议机器上，它可以很方便地分离（或扩展）case。 - wildplasser
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) 
   )
在“编程”上下文中，用于布局的方案至少应遵循一个系统，可以被读者适应。此外，值得一提的是，“这真的很主观”。 - alk
0
通过函数设计，您可以做到这样。如果您想的话，还可以使用else if结构，以使代码更紧凑，但我更喜欢这种方式。这样水平的代码是最小化的。也许我看了太多的Linux内核代码。但我认为这是内核空间中的某个人可以做到的方式。
你的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.
}
网页内容由stack overflow 提供, 点击上面的
可以查看英文原文，
原文链接