Hacker News new | ask | show | jobs
by geraldwhen 1250 days ago
It’s not, though. To confirm the method works you need to check every single comparison operator and value to ensure the range is bounded correctly. It’s code that stops you in your tracks.

Pull request denied.

1 comments

If that's the intended behaviour (having those boundaries and those results), how can you ever confirm that behaviour without checking them all?
You could have one check such as

  if(percentage < 0 or percentage > 1) 
  {
      // Throw error here
  }
Also the checks in the if statements in the linked code are redundant since they simply disregard the previous check, they could simply check if percentage < x instead of checking it's within a range sincs the previous check already proved percentage to be > x - 1/10.

To be fair though, this is the kind of code where "if it's stupid and it works, it's not stupid" applies perfectly. While I would make these changes if I had to approve a PR I wouldn't change this in a live codebase just for refactoring purposes, specially because there are better ways to show progress to a user than using Unicode characters, which I think is the real smell here.