It's pretty pathetic how many people feel the need to dunk on this bit of code just because it's not how they would write it. There's nothing really wrong with it. I'm sure the author was aware of alternative, perhaps more concise solutions using a string builder but they chose to be clear instead.
I'm pretty sure they weren't because of the redundant conditionals which simply defy logic. If there was only one check for every if statement, honestly I could give this a pass since it's at the very least simple, but by adding one extra redundant check for every statement you just created 9 new places where a bug could appear.
Furthermore, using Unicode characters to represent progress is the true smell here. There simply are better ways to do this.
In the grand scheme of things, does it matter? No. But this is Hacker News LOL, someone has to discuss it.
It has almost twice as many comparisons as necessary. The term to the left of each AND is redundant because it has already been checked by the preceding IF. It also does not guard against negative arguments. Perhaps the environment in which it is used guarantees that negative arguments cannot occur.
If I were reviewing this code I would at least ask the developer to add an assertion or contract requiring that the argument be in the inclusive range [0..1]
The choice of variable name, percentage, is also misleading. At least I suspect it is because I would expect the comparisons involving percentages to be to numbers between 0 and 100.
If lack of allocations is a requirement then one could create a static array of strings and use
int(percent * 10)
as the index. This would eliminate all of the comparisons and also throw an index out of range (in any sane language) if the value was outside the allowed range.
I vaguely suspect that this is a product of the sort of environment where you have to fill out a form in triplicate to get the static analyser to let you concatenate strings (which, to be clear, may not be inappropriate for something like this).
I do object to the variable being called ‘percentage’ tho, as it clearly isn't one.
I have no idea where all of you got the idea that percentages go up to 100. It's in the name: PER centage, meaning x/100 [0].
For instance if you want 20% that could also be expressed as a fraction such as 20/100, which turns out is the same as 2/10 or 0.2.
I do think they should remove the redundant statements in the conditions and also have an assertion that guarantees percentage to be [0, 1].
> The term "percent" is derived from the Latin per centum, meaning "hundred" or "by the hundred". The sign for "percent" evolved by gradual contraction of the Italian term per cento, meaning "for a hundred". The "per" was often abbreviated as "p."—eventually disappeared entirely. The "cento" was contracted to two circles separated by a horizontal line, from which the modern "%" symbol is derived.
This might be a little more obvious for me since my first language is derived from Latin, but anyhow it still keeps the meaning in english.
This is likely an effect of translation more than anything. While the Dutch are generally very competent English speakers and writers, their expertise tends to end the conversational level. Anything technical in its conception takes decades of intense every day use to intuit.
Source: native English speaker working in the Netherlands with a team of Dutch people. They are all really smart people, but they tend to err on the side of simple vocabulary when forced to think in English.
I'm triggered by the lack of brackets after every if-expression. Sure it looks nicer this way but the default Visual Studio code style settings will complain if you don't do it, hence I'm used to it.
I've started to remove them from my own code. It's widely mentioned as The Right Way, but I feel the reasons why are obsolete. The stated reason is always that you could forget to add braces when adding a second statement.
That was useful in a time where a text editor was "smart" when it copied your indentation to a new line. But nowadays any tooling will warn you when indentation doesn't match the bracing. The odds of people making that mistake has gone so far down, that the risk is no longer worth the reduced readability.
c7DJTLrn|3 years ago
So many big egos in software.
arp242|3 years ago
doodlesdev|3 years ago
Furthermore, using Unicode characters to represent progress is the true smell here. There simply are better ways to do this.
In the grand scheme of things, does it matter? No. But this is Hacker News LOL, someone has to discuss it.
yread|3 years ago
kwhitefoot|3 years ago
If I were reviewing this code I would at least ask the developer to add an assertion or contract requiring that the argument be in the inclusive range [0..1]
The choice of variable name, percentage, is also misleading. At least I suspect it is because I would expect the comparisons involving percentages to be to numbers between 0 and 100.
If lack of allocations is a requirement then one could create a static array of strings and use
as the index. This would eliminate all of the comparisons and also throw an index out of range (in any sane language) if the value was outside the allowed range.rsynnott|3 years ago
I do object to the variable being called ‘percentage’ tho, as it clearly isn't one.
doodlesdev|3 years ago
For instance if you want 20% that could also be expressed as a fraction such as 20/100, which turns out is the same as 2/10 or 0.2.
I do think they should remove the redundant statements in the conditions and also have an assertion that guarantees percentage to be [0, 1].
> The term "percent" is derived from the Latin per centum, meaning "hundred" or "by the hundred". The sign for "percent" evolved by gradual contraction of the Italian term per cento, meaning "for a hundred". The "per" was often abbreviated as "p."—eventually disappeared entirely. The "cento" was contracted to two circles separated by a horizontal line, from which the modern "%" symbol is derived.
This might be a little more obvious for me since my first language is derived from Latin, but anyhow it still keeps the meaning in english.
[0]: https://en.m.wikipedia.org/wiki/Percentage
yurishimo|3 years ago
Source: native English speaker working in the Netherlands with a team of Dutch people. They are all really smart people, but they tend to err on the side of simple vocabulary when forced to think in English.
pelorat|3 years ago
lucumo|3 years ago
That was useful in a time where a text editor was "smart" when it copied your indentation to a new line. But nowadays any tooling will warn you when indentation doesn't match the bracing. The odds of people making that mistake has gone so far down, that the risk is no longer worth the reduced readability.
jpnc|3 years ago
sam_lowry_|3 years ago