cif-statementconditional-statementslines

Good C-coding style for multiple lines if conditions


I am programming a project in C and have a problem: I've got a lot of if conditions and it might get really hard for other people to read. I haven't yet found a similar question on the internet.

Do you have an idea or example how to make my code more readable?

Here is the C Code:

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

Solution

  • If you want to stick with your existing code (as opposed to factor things out into inline functions), and just modify the indentation, I'm a big fan of using indent consistently. This means you can fix any source file.

    With its default options it gives you GNU indentation, i.e.:

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

    I'd say that the problem here is in fact that you are poking about illegibly in arrays. At the very least, factor out:

    uartTxSecondaryMsg[3][msgPos[3]]
    

    into a separate variable, e.g.:

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

    Obviously choose an appropriate type and variable name for foo.

    You could then separate out the limbs of the if statement into separate functions each taking foo as a parameter.