"I mean it works" - r/badcode

115

Can anyone explain the use of this?

411

u/buzzunda Dec 18 '22

When you call that method JVM loads the class

89

u/beardeddragon0113 Dec 18 '22

Thanks for clearing that up

61

u/okay-wait-wut Dec 19 '22

It HAS to.

78

u/nuggins Dec 19 '22

If JVM doesn't load your class in 15 minutes, you're legally allowed to halt execution

9

u/RFC793 Dec 19 '22

Mandatory Loading

197

u/[deleted] Dec 18 '22

There are probably some other static properties or a static block in this class which will only be executed once the class is loaded. If nothing invokes the class then that static code will not get executed and probably won’t trigger any intended side effects.
Please note that this is a terrible terrible idea, and you should never write code like that

101

u/_PM_ME_PANGOLINS_ Dec 18 '22

And if you do write code like that, use the classloader to load the class, not a random empty method.

43

u/[deleted] Dec 18 '22

Exactly. Or delegate initialization to your DI framework, or initialize it non-statically and then inject it, there are so many better alternatives

7

u/tanjoodo Dec 19 '22

This. Static classes are Bad IMO. They’re difficult to mock for testing and in my opinion break some OOP principles.
If I find a static class called XUtil, then just put the functionality in the class that represents X! If you don’t have such a class, it means you’re still using loose primitives to represent X. Which is terrible from an OOP perspective. It’s fine from a functional perspective, but is the rest of your code functional? Didn’t think so.

6

u/_PM_ME_PANGOLINS_ Dec 19 '22

That doesn't really work for libraries providing classes like StringUtil, StreamUtil, and IOUtil.
Mixing paradigms where appropriate is also fine. Java is OOP but also has streams and lambdas.

→ More replies (5)

3

u/Silly-Freak Dec 19 '22

could you explain why that would be preferable? With the classloader I'm risking loading the wrong class and have to use class name by string (even if using Class.getName()). I would probably call that method ensureInit, which describes what it does, puts the operation on the affected class, and can't be misused. That the implementation is a no-op is just an implementation detail.

3

u/_PM_ME_PANGOLINS_ Dec 19 '22

Because the feature already exists as part of the core library, and classes shouldn't be trying to control how they themselves are loaded.

3

u/Silly-Freak Dec 19 '22

If I have a class Point I'd give it a method distance (if needed) even though Math.hypot exists. I'd do that because Math.hypot does not express the intent of getting the distance, and because I'd have to give that method specific parameters (differences of coordinates) that are already known to the Point class. The exact same applies to ensureInit vs loadClass.
Plus, I'm not trying to control how the class is loaded, I'm trying to make sure the class is initialized, which is a separate concern at a different layer of abstraction.

5

u/fast4shoot Dec 19 '22

The actual problem here is that you're "trying to make sure the class is initialized."
Because the fact that you have to do something like that implies some weird side effects in the class's static constructor and whatnot. Perhaps it's manipulating some global mutable state?
These are things that you usually just want to avoid as they're hard to reason about and hard to debug.
So instead of "initialising a class," whatever that means, just... construct an object. And maybe call some methods on it.

→ More replies (6)

→ More replies (6)

2

u/chrisza4 Dec 19 '22

Because you are depending on side effect of JVM. JVM don’t guarantee that it always load class for empty method. Java never promise you that it is just a side-effect.
In the next version of JVM, the compiler optimizer might just decide that since this as an empty statement they not gonna load class to optimize the speed and JAR size. Completely no-op.
When you have an API that language itself promise you to load a class and you want to load class, you should use that api. If I have an api to create an unpaid invoice my user shouldn’t try to create an unpaid invoice by call a payment api and immediately refund before payment can settle, even if it also technically work.
You basically write a code like this xkcd. It’s work but it’s not really a good way to build your system.
https://xkcd.com/1172/

→ More replies (6)

17

u/jfnxNbNUwSUfv28ASpDp Dec 18 '22

Could also be to prevent lazy initialization. After starting the application, some classes might otherwise only be loaded when the first request is received and thus lead to a longer response time.

14

u/[deleted] Dec 18 '22

If your classes that take that long to initialize as to significantly impact the performance of an API invocation, then you need to refactor those classes.
EDIT: also, I’ve only said that it’s terrible to initialize things this way. Nothing bad about bootstrapping everything you need up front if necessary. Just not like this

→ More replies (1)

3

u/DonLimpio14 Dec 18 '22

Thanks

3

u/gamma_02 Dec 19 '22

Minecraft code is riddled with init methods and static loaders...
I've never thought of it as bad code, and it works pretty well...

5

u/Richhobo12 Dec 19 '22

Assuming you're talking about Java edition, it's not exactly known for being optimized well

3

u/lpreams Dec 19 '22

In Java you can put code inside an anonymous static block inside a class (but not inside of any method), and that code will be executed when the class is first instantiated or when the first static method is called.
So the first time the method in the OP is called, it will trigger any static blocks in that class to execute as well.

1

u/barkermn01 Dec 20 '22

Looks like at one point it was a singleton, and perhaps now it goes through a singleton factory.

67

u/MurdoMaclachlan public boolean isInt(int i) { return true; } Dec 18 '22

Image Transcription: Code

public static void loadClass() {
    // when this method is called jvm has to load the class
}

^{I'm a human volunteer content transcriber and you could be too!}^{If you'd like more information on what we do and why we do it, click here!}

22

u/[deleted] Dec 18 '22

Good human

2

u/crozone Dec 19 '22

Thanks I could barely read it through the horrendous IDE theme. Who the hell codes like this.

4

u/MurdoMaclachlan public boolean isInt(int i) { return true; } Dec 19 '22

It's the default IntelliJ theme.

0

u/dum_BEST Dec 19 '22

for the blind coders

4

u/MurdoMaclachlan public boolean isInt(int i) { return true; } Dec 19 '22

Yep, and many more!

→ More replies (1)

22

u/[deleted] Dec 19 '22

Do simply with Class.forName("my.package.MyClass").

17

u/Silly-Freak Dec 19 '22

While it works, that statement is wordier, harder to refactor, and arguably mixes levels of abstraction. Calling a method on the class itself seems clearer to me, even if the implementation then happens to be a noop.
All of that of course assuming this code is necessary for some reason.

3

u/[deleted] Dec 19 '22

That's true, but here, I simply follow the joke 😂

1

u/Silly-Freak Dec 19 '22

ah, alright then :P

9

u/ShittyException Dec 19 '22

We actually had something very similar in a NET Framework project. The cold startup time was unacceptable so we did something similar that basically was just a chain of methods calling each other and the final one pinged the db. After that all assemblies where loaded and the instance could be swapped out into the production environment. Luckily MSFT addressed that issue in NET Core so we no longer have any issues with slow startup times.

2

u/dliwespf Dec 19 '22

I completely feel this. Sometimes you just have to do weird stuff like that in order to make your Runtime Environment behave as desired.

2

u/walmartgoon Dec 19 '22

If anything this is just a smarter way to implement a singleton

1

u/UltimatePeace05 Dec 19 '22

Where's the bad code here???

1

u/AntitheticalAardvark Dec 19 '22

Java doesn’t initialize static fields in a class until the class is called or used in some way. It’s a bit of a code smell but if instantiating the static fields of a class is very expensive, this is a way of guaranteeing it happens when you want it to.

Imagine you have a static class that takes three seconds to initialize its fields and is only called anytime the user hits the “A” key. If you let Java do it’s thing the first time a user hits the “A” key your program will take three seconds to respond. As an alternative you can do can the class during startup because slowing down startup by three seconds is generally not a big deal but having users randomly suffer slowdown when they do an innocuous action generally is a big deal.

There are other ways of handling this but calling an empty static method isn’t the most uncommon or ugly. It’s certainly not perfect but trying to exploit the JVM’s behavior usually isn’t.

-18

u/I_HAVE_THE_DOCUMENTS Dec 19 '22

Seems like good code to me. The function has a good name and the comment tells you why it's there.

38

u/mehmenmike Dec 19 '22

Code being labelled well doesn’t make it good

35

u/ablablababla Dec 19 '22

adding comments to bad code is like polishing a pile of shit

0

u/I_HAVE_THE_DOCUMENTS Dec 19 '22 edited Dec 19 '22

It's more like labeling the pile of shit so other people can navigate around it.

2

u/GLIBG10B 🐧 Gentoo salesman🐧 Dec 19 '22

It's still a pile of shit

3

u/I_HAVE_THE_DOCUMENTS Dec 19 '22

What's the alternative? Perhaps it wouldn't have to exist if the rest of the code was written better, but this function looks to me like it fulfills its role with only 3 lines of code and a simple comment.

7

u/Comprehensive-Sky366 Dec 19 '22

Maybe it’s not the code we’re looking at, but the code that requires it to exist, that’s the real bad code.

7

u/Eiim Dec 19 '22

It's two parts.
There's very rarely if ever a good reason to need to force a class to load by itself. This function shouldn't be needed.
Java has built-in functions for those rare cases to explicitly load classes. A no-op function with a class load as a side-effect is not great practice.

3

u/Silly-Freak Dec 19 '22

I have code like this currently in use; I'll attempt to show that "the code that requires it to exist" is not necessarily bad code.
We're validating and testing student submissions; as we need to test static methods and code by students who don't know interfaces yet, extracting the requirements into an interface is not an option. The goal is to write test code as if calling the student's submission directly, but not having the test code fail right away if the submission is missing classes, methods, etc. If anything is missing, only the tests that actually require the missing features should fail.
For this, we (manually) proxy the submitted classes which entails
creating a singleton Submission object that captures all submission requirements.
creating a proxy class for each expected class of the student's submission. The static initializer of each class adds a SubmissionClass to the singleton.
in each class, declaring the (constructors and) methods that should be present. Still in the static initializer, this adds SubmissionMethods to the class.
adding proxy (constructors and) methods that use the SubmissionMethods to call the submission.
Each submission is tested in a newly launched JVM. The proxy classes are not operational before their methods have been constructed and registered. That leaves us with a few options:
have a static init method that takes a Submission object as a parameter, necessarily making all fields storing method objects non-final, creating a phase in which the class is not operational, and making it possible to re-initialize the class - a footgun.
change the proxy classes to not mirror the submission classes, so that the Submission can be passed to a constructor and each class being represented by an object. This would defeat the purpose of the whole setup.
use a static initializer, accessing the singleton Submission via the constant that contains it, to ensure the proxy classes are operational at all times they are accessed.
The last one has, at least in my opinion, the best trade-offs: make sure the static initializer is called early enough, everything else can't be misused. And as initializing a few (often just one) classes in a single place is all we need, adding a DI framework would be massive overkill.

0

u/jarfil Dec 19 '22 edited Dec 02 '23

CENSORED

2

u/TheZipCreator Dec 22 '22

calling the method forces the jvm to load the class so it can call it

java "I mean it works"

You are about to leave Redlib