Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

Came across this one yesterday, slightly paraphrased:

  boolean updateMsg(boolean pPassed) {
    if (pPassed) {
      incrMsgCounter();
      logger.info("Message sent");
    } else {
      logger.info("Failed to send");
      return false;
    }
    return true;
  }
Yes, the calling code actually checked the return value. The code is full of stuff like this. Somehow I've got a morbid fascination and can't stop marvelling at how grotesque it is. It even overpowers the urge to read HN.


What's so bad about this one?

Other than its returning a boolean being pointless, I mean?


As 1stamour suggested the whole function was pointless. Not a big deal by itself but if you have layers upon layers of this stuff, code quickly becomes a buggy, unmaintainable mess. There are some awesome static code analysis tools like PMD and PHPMD for measuring code quality. It's not just "all programmers think everyone else's code is crap".


The code doesn't return just any boolean. No, it returns the boolean it was passed. And the calling code checks the result, what, against the boolean it passed in, I wonder? I'd worry about unintended side-effects depending on the language...




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: