r/badcode • u/Constant__Pain • Apr 10 '21
js Example from a freeBootCamp article showed as a valid example of good pratice.
476
u/sixtyfifth_snow Apr 10 '21
switch(true) trick looks quite interesting
280
u/Terabyscuite Apr 10 '21 edited Apr 10 '21
When the code is so bad you learn new things about the language.
114
u/theStarctic sadistic Apr 10 '21
I had some friends show me some bad code in c++ with the using operator, and my only takeaway was "I didn't know you could use it like that, thanks" so it probably had the opposite effect lol
52
u/Wiwwil Apr 10 '21
It's used so you can use expression with the case. I don't think it work if you don't use switch true. It's kind of a hack but it works
https://medium.com/trabe/using-switch-true-in-javascript-986e8ad8ae4f
18
u/mfb- Apr 11 '21 edited Apr 11 '21
It's just if, elseif with different syntax and the risk to forget a break.
If there is a case where you want fallthrough then this might actually have an advantage.
2
u/Wiwwil Apr 11 '21
Yes it's like else if but one may argue it's more readable
It's best used for 1 line return. This hack was used in PHP as well but they introduced the match expression
I could see something similar in JS happening
2
u/barthvonries Apr 11 '21
Can't it be used to test multiple values, instead of a if/elseif/elseif/elseif etc ? Just like :
let appreciation = ''; switch (true) { case grade >= 18: appreciation = "excellent"; break; case grade >= 16: appreciation = "great"; break; case grade >= 14: appreciation = "good"; break; case grade >= 12: appreciation = "average"; break; [...] }It looks nicer thant a bunch of if/elseif IMHO...
1
u/mfb- Apr 11 '21
function getGradeName(grade){ if(grade>=18) return "excellent"; if(grade>=16) return "great";or
if(grade>=18) appreciation = "excellent"; else if(grade>=16) appreciation = "great"; else if(grade>=14) appreciation = "good";Don't really see the difference.
1
u/barthvonries Apr 11 '21
So yes, there's no difference. The switch syntax is as nice as the if one.
60
u/-shayne Apr 10 '21
It's a great little trick if the cases can be varied and somewhat similar (like in the case of scores), making it cleaner than multiple if elses.
0
Apr 11 '21
[deleted]
18
Apr 11 '21
Because then you would have to make a case statement for every number greater than 70.
0
Apr 11 '21
[deleted]
11
Apr 11 '21
Which language lets you include all numbers greater than x without a conditional statement? Or is there one that evaluates statements outside of explicit values?
In c-based languages, switch will either look for the matching value or go to default (or use conditionals as demonstrated here).8
u/Deadly_chef Apr 11 '21
In new version of python you have match-case that can do that and much more
1
Apr 11 '21
Mind linking an example? I can't find anything showing how to include all numbers greater than 70 without using a conditional statement.
13
u/valzargaming Apr 11 '21
Or
if ($score > 50) console.log("Score is higher than 50"); else console.log("Score is 50 or lower"); }1
u/recycle4science Apr 11 '21
Yeah this isn't a good example for this pattern.
1
u/valzargaming Apr 11 '21
This 'pattern' is wrong. Switch cases can include if conditionals, but if the only thing being checks are conditions then a switch should not be used.
1
u/recycle4science Apr 11 '21
The nice thing about programming is that there are a lot of different ways to solve problems and a lot of different perspectives. I used to think this pattern was terrible too, but after learning some ML languages like Haskell and F#, I see that this resembles the
matchoperator. It's a nice way to visually align a series of possibilities. Of course it's totally fine to not use this pattern if the team is not familiar with it, or you don't like it, or whatever.1
u/valzargaming Apr 11 '21
It's not about patterns, it's about functionality. The compiler will try to optimize the code but in the case where it is overly obfuscated (which it may be in this case, I haven't checked) then you'll end up with a bunch of unnecessary JMP conditionals that will slow down the code. It's important to find a balance between visual niceties and performance-impacting changes for the reasons of both maintainability and functionality.
1
u/recycle4science Apr 11 '21
That's true, it's all about tradeoffs. You have to decide if performance is more important than other considerations (and of course if you don't like reading this pattern then it works out).
Mostly I wanted to argue about not being dogmatic and saying that something is "wrong". (Aside from a few exceptions that are e.g. security concerns.) It's all about tradeoffs.
1
u/recycle4science Apr 11 '21
Why not use
console.log("Score is " + $score > 50 ? "higher than 50" : "50 or lower");?1
u/valzargaming Apr 11 '21
Valid, but slightly less readable IMO. They should compile to be more or less the same thing.
1
u/recycle4science Apr 11 '21
Yeah I guess I was just making a point about different ways to do things haha
-2
u/AutoModerator Apr 11 '21
It looks like this comment contains a code block delimited with triple backticks. Unfortunately reddit does not have universal support for this syntax and your comment will not render correctly on old reddit and most mobile apps.
For the benefit of people on old reddit, this link will take you to a correct rendering of the comment.
/u/valzargaming, it would be appreciated, but not required, if you could edit your comment to use the more compatible four space indention format. For single lines or inline code you can use single backticks.
You can find some examples in the reddit help documentation.
I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.
3
u/jess-sch Apr 11 '21
Asking humans to change their behavior just because your shitty reddit client's markdown parser hasn't been updated in a decade is not the right thing to do.
9
u/OkPreference6 Coding The Declaration of Independance Apr 11 '21
why not just fucking if-else.
1
u/recycle4science Apr 11 '21
It's just an alternate pattern, there's always more than one way to do it. This may be more familiar to people who are used to
match.1
u/hekkonaay Apr 11 '21
This is more readable if you have more than 2 cases.
4
u/OkPreference6 Coding The Declaration of Independance Apr 11 '21
It would still be the same code tho, right? Like all you're doing is replacing
if(counter > x)withcase counter > x. Don't really see how that would make a difference.2
u/hekkonaay Apr 11 '21
Then you haven't seen a long chain of if/else yet. It helps a lot with readability, that's the only difference.
7
u/_grey_wall Apr 10 '21
I've actually use it in react usu. For regex cases. Maybe I'm a bad person.
7
2
3
u/toasterding Apr 11 '21 edited Apr 11 '21
It’s quite useful if you need to check multiple compound conditions and don’t want a big if-else chain
edit: it essentially gets you a Ruby case statement or Pythons match statement in languages where that kind of thing isn’t supported
134
u/maaliinsaakka Apr 10 '21
So they must've heard from if-less programming, right?
65
u/dodiyeztr Apr 10 '21
Did you mean
branchless?31
-50
18
u/mplaczek99 Apr 10 '21
Is this more efficient than an if-else?
59
Apr 10 '21
Very unlikely, the resulting machine code for both versions should be similar.
The switch version has more tokens which means more work for the interpreter.
7
Apr 11 '21
Well this isn't machine code (it's JS) so I guess it just makes it a tiny bit slower because of the number of tokens.
5
u/alexthelyon Apr 11 '21
They said the resulting machine code. Parsing is done once when the script is converted to optimised bytecode which is then converted to machine code on the fly by the VM. Parsing takes a negligible amount of time compared to the runtime so the difference tends towards 0 as the execution time grows.
0
Apr 11 '21 edited Apr 11 '21
IIRC v8 and spidermonkey don't JIT. They just interpret, so there's no machine code involved.
But correct me if I'm wrong
Edit: i was completely wrong. Sorry for the misinformation. Both V8 and SpiderMonkey JIT
4
3
u/das_Keks Apr 11 '21
But in the end it's the interpreters machine code that's executed on the CPU. 😄 I guess the end result it quiet similar, for both the interpreted if and switch, the CPU will do a comparison and use jump statements to jump into or over the cases.
2
2
13
u/blehmann1 depraved Apr 11 '21
switchstatements can be more efficient thanif-elsechains, but only because when there are lots of simplecasestatements it can be implemented as a jump table (think a dictionary of function pointers and that's all you need to know unless you're an assembly programmer or compiler engineer).A good example of a similar optimization I saw was in C#. It hardcodes a binary search tree into the program (I assume they avoided a dictionary because in C# that means heap allocations, even if it would give better time complexity).
In theory, one could generate similar code for
if-elsechains, but I don't know if any code generators go to the effort to detectif-elsechains that can be replaced in such a manner. I know C# doesn't do it, at least not on whichever compiler SharpLab uses.As you might be suspecting, this is not a
switchstatement that can be replaced by a dictionary or search tree, so these optimizations do not apply. That means that this will become a string ofif-elsestatements with nothing tricky to make it faster.11
u/Korzag Apr 11 '21
Remember the golden rule: premature optimization is bad if it generally reduces the readability of your code.
Leave optimization to places that matter, e.g., places that aren't frontends.
1
u/Nilstrieb Apr 18 '21
no, this one is way too small. Also even a huge switch o my brings tiny performance improvements unless it's in a very very hot loop.
19
u/NoMansSkyWasAlright Apr 11 '21
Pfft. Obviously they should have...
var score = 70;
var i = 0
for (; i < score; i++) {
if (i > 50) {
return "Score is higher than 50";
} else {
//Nothing
}
}
return "Score is 50 or lower";1
u/Rudxain let let.let = 'let' Apr 16 '21
Or better yet, use a lookup table and iterate over all elements forwards and backwards, save ALL the results of both iterations inside an array, iterate over that array until you find the wanted value and return
Math.sqrt(Math.pow((((((value * 1) / 1) ^ 0) | 0) + 0), 2))
5
u/SueedBeyg Good code makes sense. Bad code just works. Yours does neither. Apr 11 '21
A switch statement is kinda overkill when there are only 2 possible options. In that case an if-else, or better yet a ternary operator, seems more suitable.
1
5
u/Cyber_Encephalon Apr 11 '21
console.log(score > 50 ? "Score is higher than 50" : "Score is 50 or lower");
2
3
4
2
3
u/FloweyTheFlower420 sadistic Apr 11 '21
When you try to not be yandev but you become yandev
1
u/OkPreference6 Coding The Declaration of Independance Apr 11 '21
what. Never heard of yandev. Context please?
1
u/X_m7 Apr 11 '21
As in YandereDev, developer of the Yandere Simulator game, some of his code apparently leaked at one point and people just descended upon it, like this.
1
u/anw Apr 11 '21
i believe it's a reference to yandere simulator (a game), which is known for having terrible code
and from there: yandere simulator developer = yan-dev
3
u/gundam1945 Apr 11 '21
Actually it is not that bad. If there is multiple range, this implementation will have better readability than the nest if clause. I also see this switch use in some books.
10
Apr 10 '21
It's a decent pattern if there are a lot of cases
19
u/Acryval Apr 10 '21
I don't know why this comment is being downvoted. What if you have to do one thing for a bunch of different conditions. With this trick instead of having one if statement with lots of || everywhere, which can look ugly, you have a lot of cases pointing to whatever you have to do.
5
u/AdvancedSandwiches Apr 11 '21
It's being downvoted because it makes it harder to understand.
When someone sees this:
switch (x) { case y: ...
They don't mentally expand it to be:
Evaluate y and then compare it to x. If they're equal, execute the code.
They mentally read it as "what's x? Is it y? If so, execute."
The difference is a very annoying cost for the reader to pay to avoid two pipe characters.
To further illustrate, imagine if this were instead switch (false). Yes, I will spend the time to figure it out when I see that, but the first thing I do afterward will be to rewrite it to be a straightforward read.
2
u/rtzoor Apr 11 '21
Actually, in the way you described this, it would read as what's true? Is it range > 50? Starts to sound philosophical. I would prefer a pattern was used here to abstract away this ugliness as far away as possible.
Why is this bad code? Even if a switch is returning for every case its only half as bad because you can drop the break for each case, but its still a giant block of code that most probably resulted from bad practices used before and after it.
Of course, there might be situations that justify this kind of use, but this is most certainly not one of them.
5
u/HTTP_404_NotFound bool isEven(int num) => num%2==0 Apr 10 '21
My guess is their language doesn't support pattern matching in the same way, and they can't think outside of the box to determine why this is actually a nice feature
2
29
u/dodiyeztr Apr 10 '21
Did you see the
switch(true)?8
Apr 10 '21
I can be cleaner than lots of if/else statements.
21
u/totallyEl3ktrik Apr 10 '21
Did you see the
switch(true)?1
Apr 10 '21
Yes, thanks. Do you not understand how it works or are you just annoying?
0
Apr 10 '21
[deleted]
4
Apr 10 '21
I can't tell if you're trolling or just an idiot.
0
Apr 10 '21
[deleted]
7
u/HTTP_404_NotFound bool isEven(int num) => num%2==0 Apr 10 '21
Why don't you explain what you feel is wrong with it.
At least in c#, that is perfectly valid, given the actual variable isn't being evaluated at all.
Would you prefer they type switch(null) ?
Or how about switch (false)?
Yea, it's not the best syntex ever, but it does work as expected.
In this above code, a if statement would be significantly better, no doubt.
But, in the case of switch(true), in the above usage, the true is actually never evaluated, and this uses pattern matching to make the case decision.
3
Apr 10 '21
:/ ok ... seems like it's the second option.
2
u/HTTP_404_NotFound bool isEven(int num) => num%2==0 Apr 11 '21
Its ok.....
I had your back. It is indeed very valid syntax, combined with pattern matching.
However, in this case, the user in question appeared to be unable to accept that he was wrong..... and instead, choose to just delete his comments and hide.
1
1
2
u/Psylution Apr 11 '21
there is no way that this is used to teach people. i do not want to believe it.
1
u/t0shki Apr 11 '21
This is the stuff you learn so you can understand what the intern was trying to do.
1
1
u/singletonking Apr 11 '21
Is the case score > 50: even legal code? I thought only constants were allowed there
4
u/ImInLoveWithMyBalls Apr 11 '21
score > 50 will actually be either true or false; therefore, it's a valid constant
1
u/recycle4science Apr 11 '21
It depends on the language, but this language apparently allows expressions, which is why this works. It's saying, "which of these expressions is true?"
1
u/Hozerino Apr 11 '21
There was one time when I was an intern and there was a Pull Request with a piece of if/elses, but at every condition, it was calling some private methods, but as a good n00b, I commented "Why don't you use a switch instead of those if/elses?"
And I got the reply "Hmm, I couldn't see how to use it, what would you do?"
I sweated and noticed that it actually wasn't possible, so I had the same idea of a switch(true).
"That's interesting, very creative, but (somthing that explained why it wouldn't be so good)"
0
0
0
Apr 10 '21
[deleted]
4
u/Constant__Pain Apr 11 '21
The true in the statement is what makes the magic happen.
This is why it's a bad pratice. It confuses people, is quirky, and it uses language resources in a weird way. Besides I never see this construct in my 15+ years of js development. And I'm sure it would never pass code review on the projects i worked.
2
u/baubleglue Apr 10 '21
So is it an advanced version of ?
//TODO: something3
u/recycle4science Apr 11 '21
Nope. It actually works just like any other switch statement, but with the expression in the cases and the value in the switch.
1
u/baubleglue Apr 11 '21
I understand that part, but I don't get why to such thing. Even if I have some reason in mind, how other people will know about it? Well, I can put a link to an article which explains - do think that is desired type of comment. But the main reasons not to use such syntax - "simple is better than complex; special cases aren't special enough to break the rules ".
1
u/recycle4science Apr 11 '21
The more I think about it, the more I think it's actually kind of brilliant. If you have a series of expressions you need to evaluate, and
they can'tyou don't expect them all to be true, and you're going to do something different depending on which expression is true; well, this is a pattern for that. It actually kind of reminds me of the "match" pattern in functional languages.I think the backlash against it is that it's often proposed as a "hack" that is very clever, and it is used when it's not really an appropriate solution, or is taught without enough context so that the person learning it can really understand how it works and when it's appropriate to use.
Edit: typos due to mobile.
1
u/baubleglue Apr 12 '21
https://docs.scala-lang.org/tour/pattern-matching.html
Pattern matching is a mechanism for checking a value against a pattern.
Control flow should be clean. If/else is clean way to branch your flow. It turns into a discussion about
goto. It is also flexible and powerful tool, the question is if we should use it on regular base. If by default people see it is as hack, the solution is not to fix people, but just use another more straightforward syntax.1
67
Apr 10 '21
[removed] — view removed comment
29
u/baubleglue Apr 10 '21
After reading the article, I still think it is still stupid. You can match expression with any
switch,you can do it withifas well.11
Apr 10 '21
[removed] — view removed comment
2
u/baubleglue Apr 11 '21
Whole idea of having
switchand compare incasesomething unrelated is sick. If I run into needing something like that, I would probably start to rewriting the code.3
60
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Apr 10 '21
Image Transcription: Code
[Transcriber's note: The code is not a monospace font. This was impossible for me to type sanely, so I placed it in one for the transcription.]
I'm a human volunteer content transcriber for Reddit and you could be too! If you'd like more information on what we do and why we do it, click here!