Skip to main content
typos, minimal editing
Source Link
ern0
  • 3.2k
  • 30
  • 43

I've just run into it. We have items, which

  • should be checked for several things,
  • checks require some preparation,
  • we should apply cheap checks first, then go with expensive ones,
  • orsome checks depends each otherothers,
  • whichever item fails on any check, it should be logged,
  • if the item passes all the checkchecks, it should be passed to further processing.

Watch this, without continue:

foreach (items) {

   prepare check1
   if (check1) {

      prepare check2
      if (check2) {

        prepare check3
        if (check3) {
          log("all checks passed")
          process_good_item(item)
        } else {
          log("check3 failed")
        }

      } else {
        log("check2 failed")
      }

   } else {
      log("check 1 failed")
   }    
}

...and compare with this, with continue:

foreach (items) {

   prepare check1
   if (!check1) {
      log("check 1 failed")
      continue
   }

   prepare check2
   if (!check2) {
      log("check 2 failed")
      continue
   }

   prepare check3
   if (!check3) {
      log("check 3 failed")
      continue
   }

   log("all checks passed")
   process_good_item(item)
}

Assume that "prepare"-s are multiple line long each, so you can't see the whole code at once.

Decide yourself, which is

  • less complex, have a simpler execution graph
  • have lower cyclomatic complexity value
  • more readable, more linear, no "eye jumps"
  • better expandable (e.g. try to add check4, check5, check12)

IMHO Misra is wrong in this topic.

I've just run into it. We have items, which

  • should be checked for several things,
  • checks require some preparation,
  • we should apply cheap checks first, then expensive ones,
  • or checks depends each other,
  • whichever item fails on any check, it should be logged,
  • if the item passes all the check, it should be passed to further processing.

Watch this, without continue:

foreach (items) {

   prepare check1
   if (check1) {

      prepare check2
      if (check2) {

        prepare check3
        if (check3) {
          log("all checks passed")
          process_good_item(item)
        } else {
          log("check3 failed")
        }

      } else {
        log("check2 failed")
      }

   } else {
      log("check 1 failed")
   }    
}

...and compare with this, with continue:

foreach (items) {

   prepare check1
   if (!check1) {
      log("check 1 failed")
      continue
   }

   prepare check2
   if (!check2) {
      log("check 2 failed")
      continue
   }

   prepare check3
   if (!check3) {
      log("check 3 failed")
      continue
   }

   log("all checks passed")
   process_good_item(item)
}

Assume that "prepare"-s are multiple line long each, so you can't see the whole code at once.

Decide yourself, which is

  • less complex, have a simpler execution graph
  • have lower cyclomatic complexity value
  • more readable, more linear, no "eye jumps"
  • better expandable (e.g. try to add check4, check5, check12)

IMHO Misra is wrong in this topic.

I've just run into it. We have items, which

  • should be checked for several things,
  • checks require some preparation,
  • we should apply cheap checks first, then go with expensive ones,
  • some checks depends others,
  • whichever item fails on any check, it should be logged,
  • if the item passes all the checks, it should be passed to further processing.

Watch this, without continue:

foreach (items) {

   prepare check1
   if (check1) {

      prepare check2
      if (check2) {

        prepare check3
        if (check3) {
          log("all checks passed")
          process_good_item(item)
        } else {
          log("check3 failed")
        }

      } else {
        log("check2 failed")
      }

   } else {
      log("check 1 failed")
   }    
}

...and compare with this, with continue:

foreach (items) {

   prepare check1
   if (!check1) {
      log("check 1 failed")
      continue
   }

   prepare check2
   if (!check2) {
      log("check 2 failed")
      continue
   }

   prepare check3
   if (!check3) {
      log("check 3 failed")
      continue
   }

   log("all checks passed")
   process_good_item(item)
}

Assume that "prepare"-s are multiple line long each, so you can't see the whole code at once.

Decide yourself, which is

  • less complex, have a simpler execution graph
  • have lower cyclomatic complexity value
  • more readable, more linear, no "eye jumps"
  • better expandable (e.g. try to add check4, check5, check12)

IMHO Misra is wrong in this topic.

Source Link
ern0
  • 3.2k
  • 30
  • 43

I've just run into it. We have items, which

  • should be checked for several things,
  • checks require some preparation,
  • we should apply cheap checks first, then expensive ones,
  • or checks depends each other,
  • whichever item fails on any check, it should be logged,
  • if the item passes all the check, it should be passed to further processing.

Watch this, without continue:

foreach (items) {

   prepare check1
   if (check1) {

      prepare check2
      if (check2) {

        prepare check3
        if (check3) {
          log("all checks passed")
          process_good_item(item)
        } else {
          log("check3 failed")
        }

      } else {
        log("check2 failed")
      }

   } else {
      log("check 1 failed")
   }    
}

...and compare with this, with continue:

foreach (items) {

   prepare check1
   if (!check1) {
      log("check 1 failed")
      continue
   }

   prepare check2
   if (!check2) {
      log("check 2 failed")
      continue
   }

   prepare check3
   if (!check3) {
      log("check 3 failed")
      continue
   }

   log("all checks passed")
   process_good_item(item)
}

Assume that "prepare"-s are multiple line long each, so you can't see the whole code at once.

Decide yourself, which is

  • less complex, have a simpler execution graph
  • have lower cyclomatic complexity value
  • more readable, more linear, no "eye jumps"
  • better expandable (e.g. try to add check4, check5, check12)

IMHO Misra is wrong in this topic.