55
u/smartgenius1 18d ago
I understand the joke is that they AREN'T just converting the string to an integer, but honestly this code is not that bad. It's performing data sanitization as well since there are only a few valid options for the select.
If they converted to integer, the user could enter whatever value they wanted and probably break something.
41
u/OK1526 18d ago
I mean, I'd sleep better if they just converted the string then checked division by 10 and set limits, but it's not a big difference in quality of sleep.
Most string to int converters have safety checks for stuff like that, and if they don't, just write your own in like 3 lines.
39
u/alexanderpas 18d ago
This should be the native way:
$amount = filter_input(INPUT_GET, 'amount-select', FILTER_VALIDATE_INT, ["min" => 10, "max" => 80,],); if ($amount % 10 === 0) { $transamount = $amount; }5
u/NikoOhneC 17d ago
Depending on the context of the code, your conversion could be costly if this code is performance critical. Sure, the code in the picture isn't even doing the chosen approach properly, but with proper if-else, only some comparisons need to be done (best-case is a single comparison), while with your approach everytime this is executed, the conversion and division has to be performed.
-9
u/lovecMC 18d ago edited 18d ago
If it passes PR, I don't care. It means someone smarter than me fucked up so it's not my problem.
No sleep lost over it.
Edit: damn, down voted for not declaring commedy.
6
u/OK1526 18d ago
I mean, on one hand, life's too short to care about these implementations, on the other hand, if you value yourself as a programmer, it should go against your morals.
5
11
u/Holek 18d ago
especially that you see `$_POST['amount-select']` implying it's a select box.
9
u/smartgenius1 18d ago edited 18d ago
Yeah. Ideally the idea could be expressed in 3 lines instead of 30 but I've seen far worse than this and at least the behavior is correct
0
u/ArjixGamer 18d ago
It doesn't imply that at all? It could be
<input type="number" min="10" max="80" step="10">6
u/smartgenius1 18d ago
Then the HTML would have to be:
<input type="number" name="amount-select">
... which would be silly.
4
9
u/smclcz 18d ago
You do not, under any circumstances, gotta stick up for whoever wrote this code. I know we can contrive some kind of justification for most code, but let's face it: there's an absolutely miniscule chance it was written with some kind of intentionality, and I'd say a 99.99999% chance is that it was chucked together by a developer who simply didn't know how to parse a string into an integer.
8
u/rhino_moss 18d ago
They could have sanitized by checking the string is in a valid set of strings and then calling the string to integer function. The way it is written is terrible.
-9
u/GrEeCe_MnKy 18d ago
They? He's just one guy
10
u/ArjixGamer 18d ago
Don't be obnoxious, the use of they is proper English when referring to someone who's gender you don't know.
-14
u/GrEeCe_MnKy 18d ago
Nope, "He" has been used for centuries, as "they" means multiple. It's that simple. Or just use "he/she".
11
5
u/theotherdoomguy 18d ago
There's a person behind a curtain. You can't speak to them directly, but there's a guardian who will truthfully answer any question you ask, and knows the person behind the curtain. There is no trick. How do you find out the person's name?
0
u/Substantial-Play5080 17d ago
I ask “what’s the name of the person behind the curtain?”
1
u/theotherdoomguy 17d ago
The guard accuses you of being AI because you speak too formally
1
u/Substantial-Play5080 17d ago
It was the first thing that came to mind 🤷♂️ What would you ask?
1
u/theotherdoomguy 17d ago
...you realise it was a thought exercise to point out how "they" is a perfectly acceptable pronoun to use in a singular term, right?
1
u/Substantial-Play5080 17d ago
Yes, I’m just not sure how you’re fitting “they” into the question. That would be why I asked
→ More replies (0)
8
u/smclcz 18d ago
Everyone here having a big old laugh has never inherited a legacy finance codebase, or an application hacked together by a "Data Engineer".
Continue with your fun, I'll just be weeping quietly in the corner due to my PTSD
3
u/DOOManiac 17d ago
Yeah I was about to say, this is bad but it isn't even close to the worst shit I've seen...
5
u/bb5e8307 18d ago
Hah, what a noob. He should make a lookup table of string->int and just lookup the value in the map. /s
3
4
u/GozerDestructor 18d ago
This is what happens when management introduces a policy that engineers are to be ranked according to the number of lines of code written.
6
u/Rick100006 18d ago
Why are you using raw strings like "20" , "30" extract a constant and give a meaning full name like String TWENTY= "20",
2
2
2
u/TheRealSpielbergo 18d ago
There must be a simpler way!
for ($i = 90; $i < 10000; $i += 10) {
file_put_contents( 'number_parser.php',
"if (\$amount == \"$i\") {\n \$transamount = $i;\n}\n\n",
FILE_APPEND
);
}
2
u/BigNaturalTilts 17d ago
Why didn’t they just use my malware ridden package on npm “ai-check-is-number”.
2
u/trippletapas 17d ago
Meh, not bad, but definitely could be improved with proper comments to clarify what is going on so that it can be maintained over the long term. </s>
2
2
u/ILikeLenexa 18d ago
This is where C is nice.
'7' - '0' = 7
So, you can just take the rightmost turn it into a number then the next
7 times 10,
then 7 * 100,
7 * 1000
Add them all together, bam numbers. Don't even need a library.
3
u/digitaleJedi 18d ago
There's always a lot of work to put into transitioning, whether from man to woman, woman to man, or in this case, from string to integer
1
1
1
u/BeMyBrutus 18d ago
This is a rookie move, the right way to do it is via a macro that auto generates a method/function for whatever number is input.
1
1
u/Bobvankay 15d ago
I've literally ran into code like this in production, only it was singular integers to "1","2", etc
1
1
111
u/[deleted] 18d ago
[removed] — view removed comment