[jpnc]

Is this literate programming?

[sam_lowry_]

It's been extensively discussed on twitter, and the general conclusion seems to be that yes, this particular snippet is good code.

[Freak_NL]

The only thing that jumps out at me is this:

    if (percentage == 0)
        return […];
    if (percentage > 0.0 && […]

Can a double have a value that is larger than 0 but smaller or equal to 0.0? I would have expected '> 0' instead.

[kadoban]

In any language I can think of, 0 and 0.0 are the same, once you're comparing them against a double.

[DoingIsLearning]

Technically what is happening behind the scenes is that for most languages the compiler/interpreter will promote the integer to a double to avoid foot guns.

Nevertheless integer comparisons with any kind of floating point is not a wise choice.

The idiomatic way to compare a double would be to take into account whatever is the double precision epsilon for that language. Or just use the greater/less than like they have in the subsequent if statements in the original code snippet.

[LudwigNagasena]

Discussed by whom? By people who deem every non-built-in data structure as "too clever" for maintenance?

[geraldwhen]

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.

[icoder]

If that's the intended behaviour (having those boundaries and those results), how can you ever confirm that behaviour without checking them all?

[doodlesdev]

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.