Java bad practice doing: new... ().doSomething()?

Question

I just saw a piece of code that had some classes with only one method. I picked an examples:

public class TempDirCleanupProcess {
  public void cleanup(final File directory) {} 
}

Then, later on in the code the method was called the following way:

new TempDirCleanupProcess().cleanup(tempDir);

Now I am wondering if this is a bad practice because I have seen such "behavior" only with static methods before. Any oppinions on that?

With non static methods, every call your TempDirCleanupProcess is constructed anew. If it happens often or it takes much resources you should consider injecting a cleanup service. Static methods are class methods, so they do not have this problem — Mr. Wrong, Apr 17 '19 at 10:51
Possible duplicate of [new className().methodName(); VS className ref = new className();](https://stackoverflow.com/questions/31607335/new-classname-methodname-vs-classname-ref-new-classname) — Sudhir Ojha, Apr 17 '19 at 10:52
The difference is that static methods cannot be overridden, whereas non-static methods can. — MC Emperor, Apr 17 '19 at 10:53
@MCEmperor but if you're writing `new TempDirCleanupProcess()`, you're getting that exact class' methods, not an override. — Andy Turner, Apr 17 '19 at 10:54
Using static methods is not object oriented, introduces hidden hard coupling, makes testing a nightmare and is considered by many OOP purists an anti-pattern. personally I would only use static methods as named constructors. — Svetlin Zarev, Apr 17 '19 at 11:04
with "new" keyword the constructor method will be called recursively over all subclasses until object. It will also allocate memory for your object on Heap. It is an expensive operation for just calling a method. — neoexpert, Apr 17 '19 at 11:05
One may say it's too broad, or not clear enough, but it's definitely not opinion-based. I am voting to reopen it. — Andrew Tobilko, Apr 17 '19 at 11:30
To clairify this: I want to know if this is any Software engineering related Bad practice mit how anyone of you would handle this! — ItFreak, Apr 17 '19 at 11:47
@ItFreak what did you mean by "anyone of you would handle this"? If you can rewrite that piece, we recommend you go with a static method. — Andrew Tobilko, Apr 17 '19 at 17:11

Andrew Tobilko · Accepted Answer · 2019-04-17T11:27:26.277

Sure, it could be refactored into a class with a static method. It would obviate the need for creating an instance every time one needs to call the method. In this particular case with no additional context given, a static method would be a nicer solution.

However, don't forget a class can hold a state and a single method may change that state and return a reference to the current object.

public class Builder {
  // state

  public Builder buildPart(T part) { 
      // update the state
      return this;
  } 

}

It would resemble a variation of the builder pattern and make sense.

return new Builder();
return new Builder().buildPart(partA);
return new Builder().buildPart(partA).buildPart(partB);

I can also think of an extremely poor design where this would be leaked out from cleanup, so a reference to that new TempDirCleanupProcess() wouldn't be lost after the line is executed.

Ori Marko · Answer 2 · 2019-04-17T11:12:04.787

3

It looks like a standard static method, but we don't see all the details

So maybe when you are creating the object you are also creating instance members that are used in the method cleanup and you must create the object in order to make them available

edited Apr 17 '19 at 11:12

answered Apr 17 '19 at 11:06

Ori Marko

56,308
23
131
233

score 1 · Answer 3 · answered Apr 17 '19 at 11:04

1

Another approach would be to have a Directory class like such

Directory temp = new Directory('path/to/file');
temp.cleanup()

This also allows you to inherit the Directory class in some other class that requires all of these utility functions.

That being said, a utility function in a class like yours should be static.

answered Apr 17 '19 at 11:04

Louis Christopher

308
3
17

1

It must not be static because it would introduce hard & hidden coupling and will make testing a nightmare. – Svetlin Zarev Apr 17 '19 at 11:05
@SvetlinZarev you could use `PowerMockito.mockStatic`. – DodgyCodeException Apr 17 '19 at 11:15
Of course you can, but if you have to cheat, then your design is shit. – Svetlin Zarev Apr 17 '19 at 11:27
@SvetlinZarev Are you saying the method itself would be a testing nightmare or that using the method in other code would be a testing nightmare? If the former, it shouldn't be any harder to test than testing an instance method—assuming the method is effectively self-contained. If the latter, you could always abstract the call to the method, allowing you to mock it at the call site (though this may not be acceptable/desirable). – Slaw Apr 17 '19 at 11:27
3

I'm saying that using static methods is bad practice. The simplest real world case is introducing a hidden dependency to `System.getProperty` or the like - imagine a test that depends on system properties and it fails because some other test had set some property. `you could always abstract the call to the method` -> Of course you can abstract it using an interface, but then it's not a static method anymore, is it ? Then you have a polymorphic call, not a call to static method. You have a dependency to an abstraction, not a concrete class. – Svetlin Zarev Apr 17 '19 at 11:31
@SvetlinZarev are you saying that _all_ static methods are bad (including `Math.abs`, `Collections.unmodifiableMap`, etc) or just a specific type of static method? If it's the latter, what would make a static method bad? – DodgyCodeException Apr 17 '19 at 15:44
IMO almost all static methods are bad. For instance in Rust you do not have `Math.abs`, but call `123.abs()` on the number itself (same as min/max/sqrt, etc) `Collections.unmodifiableMap` is a named constructor, which is the only valid use for a static method. The issue here is not with the method itself, but the lack of real unmodifiable collections(see the rust-lang approach). Static methods are bad because they are not object oriented - they are procedural code and are born from procedural way of thinking. – Svetlin Zarev Apr 17 '19 at 17:07
1

@DodgyCodeException This guy has a very nice blog about OOP (although it's very radical at times): https://www.yegor256.com/ He has very nice articles about oop, static methods and the getter/setter antipaterns. – Svetlin Zarev Apr 17 '19 at 17:20

Java bad practice doing: new... ().doSomething()?

3 Answers3