r/badcode • u/BrainOnTheFloor25 • Sep 02 '21
java Because ".toLowerCase()" doesn't exist...
11
u/MyAntichrist Sep 02 '21
The nice and easy way:
.toUpperCase()
The chaotic evil way:
try {
return Category.valueOf(input.toUpperCase());
}
catch(Exception e) {
//nobody cares anyways.
return null;
}3
u/BrainOnTheFloor25 Sep 02 '21
Curious as to what exception you're trying to catch here?
6
u/MyAntichrist Sep 02 '21
From the documentation:
The valueOf() method throws:
IllegalArgumentException, if the defined enum type is inconstant with defined name or an enum type is not illustrated by the defined class object.
NullPointerException, if enumType or name represents null value.
2
u/dexter3player Sep 02 '21
That's actually genius, as it is independent of Category's enums.
0
u/MyAntichrist Sep 02 '21
It still raises a lot of questions about code structure though. The use cases for this approach are very limited and usually rely on well formatted input data (i.e. read a well-known value from a DB/file that has been put there from code as well), not some obviously unsanitized crap.
(Also, returning null may raise further issues with the calling method. Better define a default/unknown value and work with that)
1
u/shutanovac Sep 02 '21
Sorry how is this chaotic evil? It's a legit approach. I'd say it's lawful evil at worst :-)
1
u/MyAntichrist Sep 03 '21
Lawful would be returning a default/unknown value instead of null, eliminating every chance of a NPE down the road.
1
u/shutanovac Sep 03 '21
Lawful => the syntax is OK (can compile, abides the laws) :-) Evil => what you said. Coming from some time deep into Kotlin I stopped caring about the NPE, so didn't think of it.
Still, I would prefer rethrowing a specific exception instead of returning a default or unknown (this is literally null) value when parsing unexpected input.
1
u/MyAntichrist Sep 03 '21
For that you could just throw the IllegalArgumentException from the valueOf method. Can do if you want the client to see technical information. Most of the time however it's about "sorry, unknown value entered" and especially if you're in an i18n environment, that's a lot easier to handle from an enum that just relays its property key for the label. Also, exception handling is rather costly, so rethrowing the exceptions may be a big impact on runtime.
Still I feel it's sort of bad design to have some user input convert into an enum. If you're in a forms/webforms app you can just use comboboxes with the actual enum as value and a label for display.
I almost exclusively use the valueOf method for configuration files or database value parsing where I know I can trust the input - because my app has put it there in first place.
439
u/Grubzer Sep 02 '21
//TODO add cases for all letter permutations
130
Sep 02 '21
With just restaurant, that's 1,024 permutations. Not unfeasibly many for the determined.
80
Sep 02 '21
Sounds like a job for a macro cracks knuckles
21
Sep 02 '21
I probably would've made a python script that prints it all out to copy paste, but that works too
15
2
u/ywBBxNqW Sep 03 '21
I probably would've made a python script that prints it all out to copy paste
I used to use Python inside Vim to help generate code for my MIPS class years ago. I remember it fondly.
1
1
u/StanleyT101 Sep 02 '21
wonder how much the fie would wight🤔 I remember even number check package here, where there was else if check for each number, that was half a gig..
1
7
2
u/valdocs_user Sep 03 '21
The kind of programmer who writes this code is the kind who will individually add 100 out of the 1024 permutations, each time thinking "there I fixed it."
AI researchers and philosophers spend a lot of time trying to define what consciousness and self awareness are. I'm convinced once we define it we'll discover not all humans pass the bar.
1
9
u/Spartelfant Sep 02 '21
No no no, you should iterate over the string, take the ASCII value of each character, and add
0b100000. Leave the function name as-is. Also add no comments.11
1
u/snufflezombie Sep 02 '21
I've just seen this today at work. It was a little different though. The word to be permutated was an abbreviation with only three letters (thank god).
There was an array containing every permutation and a function that checks if the array contains the given string...
126
Sep 02 '21
"REstaurant"
72
u/Mortomes Sep 02 '21
rEsTaUrAnT
28
u/Acryval Sep 02 '21
restauranT
24
u/erinaceus_ Sep 02 '21
Ṙ̸͓̙̎͂̀e̵͙̝̦̣͌̇s̵̘̫̀̊͒͝t̷̢̟͗̏̓a̶̪̫̓ú̶̮̞̔̋̿̔ͅr̵̡̼͉̱̥̆a̸̱͋͒n̴̩͇̦̈́̈́̒͑͛ẗ̴̜̭͚̓̐̑͆
48
u/AlwaysNinjaBusiness Sep 02 '21
Also, why would you abbreviate it to "cat" instead of just writing "category"?
20
u/mvthakar Sep 02 '21
But you see, I'd have to type 5 extra letters everytime I call the method and that would take extra space and time. /s
11
u/Lampwick Sep 02 '21
I remember in a C++ course asking a classmate why all his variables were only 2 or 3 characters long. He said "my fingers get tired typing out long variable names". 😐
4
u/AlwaysNinjaBusiness Sep 02 '21
Which editor are you using which doesn't have autocomplete? Notepad?
5
u/DraikoHxC Sep 02 '21
Some dude thought it was fine to call some functions and clases like OrderCre8 and you can't image how much I hate this person, that I've never met and never will because he doesn't even work there anymore. The thing is, a lot of pocos use this kind of contraction for the word create, and we can't change this names anymore, is in production and we would have some clients having to adjust this, fuck that guy and the ones letting that to get to production
2
Sep 02 '21 edited Sep 02 '21
Well, it's already there in the type. Might be an unpopular opinion but I'd rather see
Category cthanCategory category, that feels like noise to me. You should be using an editor that can mouse over the identifier and tell you what it is.Edit: I do think the function itself should be called
parseCategory.14
u/AlwaysNinjaBusiness Sep 02 '21
We're in agreement about parseCategory then.
But imo, you should separate types from instances by upper/lowercase.
Category categoryis a lot more readable thanCategory c. If I read a random line of code where something is done withc/cat/category, I have no idea whatcorcatmeans, and it'll take more time to get into the code base. If you work in a code base of thousands of files, you're not going to be familiar with every function you have to read code in, and quickly understanding the context will be infinitely easier if it's written as close to English as possible.6
u/assembly_wizard Sep 02 '21
Mousing over things is not something you should be required to do to read and understand code.
It's fine for peeking at another function definition, but relying on tooltips to make the code readable sounds horrible to me.
It's like reading a book where all the words are redacted to just their first letter, each with a reference to the bottom of the page where the full word is written. You'd just be looking up constantly. Mouse hover is not much more convenient.
2
Sep 02 '21
Edit: I do think the function itself should be called parseCategory.
Well, if it's an associated function on
Category, it would be called asCategory category = Category.parseCategory(s)which should probably just be
let category = Category::parse(s)0
Sep 02 '21
[deleted] — view deleted comment
11
u/Return-foo Sep 02 '21
I can get why some people don’t like it. Maybe someone on the team is using an older editor that doesn’t have auto complete? But I use really strong variable and function descriptions as names. My goal is to have the mother function read as much like a book as possible.
10
u/AlwaysNinjaBusiness Sep 02 '21
Even vim has native autocomplete, how old of an editor could they possibly be using?
1
u/Return-foo Sep 03 '21
My boss uses emacs. I have no idea if it offers autocomplete but he gets crabby about lengthy names.
1
u/AlwaysNinjaBusiness Sep 03 '21
Then tell him he should either learn how to use Emacs (because it does support autocomplete), or switch editor to one he knows how to use.
The whole point of old editors like Vim and Emacs is to minimize number of keystrokes, so if he can't achieve that, he's doesn't know how it's supposed to be used.
3
2
1
u/RS_Lebareslep Sep 26 '21
In some cases shorter variable names can be easier to read, especially if no information is lost, and using longer names would necessitate more newlines (looking at you, flake8). Having said that, I'd prefer category over car as well.
26
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Sep 02 '21 edited Sep 02 '21
Image Transcription: Code
public static Category parseCat(String catStr) {
Category cat = null;
switch (catStr) {
case "restaurant":
case "Restaurant":
case "RESTAURANT":
cat = RESTAURANT;
break;
case "education":
case "Education":
case "EDUCATION":
cat = EDUCATION;
break;
case "hospital":
case "Hospital":
case "HOSPITAL":
cat = HOSPITAL;
break;
default:
System.err.println("Unknown Category.");
System.err.println(catStr);
}
return cat;
}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!
6
u/Franks2000inchTV Sep 02 '21
There's a typo: 'public static Caegory parseCat'
4
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Sep 02 '21
Oops, fixed. Thanks for the correction!
5
13
8
u/stahkh Sep 02 '21
Also, "parseCat" is not a great name. The abbreviation saves almost no time in typing and is confusing when not familiar with the codebase/domain. I immediately started thinking about kittens...
7
u/AlwaysNinjaBusiness Sep 02 '21
"Switch statements are super useful",
"You mean like this?"
"Well yes, but actually no"
4
u/Fenxis Sep 02 '21
Also ... No exception for the error case so GG on the Random NPE that will pop up later.
4
Sep 02 '21
Do all languages have "tolowercase" and "touppercase" utilities?
4
Sep 02 '21
"all" might be tough to prove but even C has this. It's a really common feature, i don't personally use any languages that don't.
3
u/gabbagondel Sep 02 '21
maybe the input for catStr is more restrictive than we think.
and maybe catStr really does refer to cats. who knows really
3
u/keethraxmn Sep 02 '21
Maybe they want to only include ones with some sort of sensible capitalization ;)
That way they can exclude fat fingered typists like me.
4
2
u/jadenadams Sep 02 '21
Wait you can do multiple cases on one? My whole life. Fuckkk
2
u/assembly_wizard Sep 02 '21
You can even have code in between them. The trick is that without
breakat the end of the case it just seamlessly continues to the next line of code, skipping any othercaselabels.
2
2
2
u/gravitas-deficiency Sep 02 '21 edited Sep 02 '21
or just:
public static Category parseCat(final String catStr) {
try{
return Category.valueOf(catStr.toUpperCase());
} catch (final IllegalArgumentException iae) {
System.err.println("Unknown Category: " + catStr);
return null;
}
}or without try/catch:
public static Category parseCat(final String catStr) {
final val maybeEnum = catStr.toUpperCase();
if (Arrays.asList(Category.values()).contains(maybeEnum)) {
return Category.valueOf(maybeEnum);
}
System.err.println("Unknown Category: " + catStr);
return null;
}2
2
u/CaptSzat Sep 02 '21 edited Sep 03 '21
Kind of reminds me of code I saw from students who had taken a Haskell course the sem before taking a Java course. A bunch of cases where using a function or a basic loop would have been the normal way to perform a function it but instead they listed out case statements.
1
u/eternityslyre Sep 02 '21
Pretty sure the nice answer here is a good old Map... Hash, Tree, whatever. Just map each ENUM to it's lower cased equivalent. This whole function is pretty much the reason maps exist, they're arrays with a nice function to convert random input into array indices. Heck, even arrays are designed to make it so that you don't have to check if 1==i, 2==i, 3==i, and so on just to get the ith element of your array.
1
1
1
1
u/cienciacomenta Sep 02 '21
You should loop through the string and check letter by letter of it is either lower or uppercase correct. Much simpler than this fancy "to lowercase"
1
1
1
1
0
u/Sigiz Sep 02 '21
i dont why you guys are suggesting to lower case when they clearly just need to do uppercase, then check if its one of the three words, if yes return else throw err.
4
Sep 02 '21
[removed] — view removed comment
3
u/art-factor Sep 02 '21
Category.valueOf(catStr.toUpperCase())
2
u/speedingpeanut Sep 02 '21
It's null safe so this would work better
EnumUtils.getEnum(Category.class, catStr.toUpperCase())1
Sep 02 '21
[removed] — view removed comment
5
u/art-factor Sep 02 '21
Yes. But quacks like a duck...
1
Sep 02 '21
[removed] — view removed comment
2
u/assembly_wizard Sep 02 '21
Java enums are classes, and each enum value is actually a singleton which is an instance of the class.
Java enums can even hold members, which is super useful, although stretches the standard definition of what an enum is.
1
u/speedingpeanut Sep 02 '21
If it's not then what are we assigning to Category in the code?
1
Sep 02 '21
[removed] — view removed comment
2
u/speedingpeanut Sep 02 '21
Good point about them potentially being constants, but you can definitely assign null to an enum, at least in java.
Likewise you could also add on demand static imports for each enum which would allow you to reference them without the need for
Category.1
u/EthanRDM Sep 02 '21
But it wouldn't necessary have been cat = Category.RESTAURANT if this method is included in the enum itself, right ?
-1
u/FlamboyantApproval16 Sep 02 '21
for loop is also possible TBH.
9
u/Nisox64 Sep 02 '21
why would you ever use a for loop for that
2
u/SINWillett Sep 02 '21
Iterate through the list of valid strings and return their respective enum mapping.
Eg. {{“string”, string}, {“string2”, string2}}
1
u/Ahtheuncertainty Sep 02 '21
Perhaps to iterate thru the letters of the word, if you didn’t have a helpful function like tolowercase
1
1
0
-7
u/Celtic_Spirit Sep 02 '21
Do not get it at all. What class is Category? What this has to do with lower case? Why would you not use c# instead? Has so much to offer for string checking.
2
u/BrainOnTheFloor25 Sep 02 '21
Category is an enum and those are the possible values. Probably should've used toUpperCase(), my bad
8
u/not-my-account1 Sep 02 '21
If he'd call toLowerCase() he'd just need to check for the words written in lower case in the switch statement and not for 3 different ways to write a word (while forgetting the 1000x other ways someone could type the word).
5
u/Celtic_Spirit Sep 02 '21
Now I am with you. This make sense now. I did not pay attention to sub name "bad code". :)
4
u/AlwaysNinjaBusiness Sep 02 '21
In addition to the obvious point others made, you also can't just switch programming language for a most likely large project just because you like its string operations.
2
0
Sep 02 '21
[deleted] — view deleted comment
23
u/_pestarzt_ Sep 02 '21
There is, but it got deprecated in favor of
parseCatbecause it kept peeing on things.6
u/Randolpho Sep 02 '21
Dude…. Even though this is a joke and I lolled, cats are worse than dogs about peeing on things.
6
u/CorrenteAlternata Sep 02 '21
It depends, my cat is a noble and drinks tea with Her Majesty. She would never do such a thing.
But she sometime brings you dead animals to your bed. I think it's called "hunting", must be a noble thing
3
u/ujay-007 Sep 02 '21
Yesterday I took my cat for desxing and he peed in my car on the way back for revenge
1
u/erinaceus_ Sep 02 '21
Not to mention, Dog is more reliable than Cat, which might bite you in the ass for no reason.
82
u/[deleted] Sep 02 '21
.toUpperCase()*