Java - Getter/Setter, behavior and Interfaces

Question

I have a question, a little bit theoretical:

Assume, I have the following classes :

interface ReportInterface {
     void execute();
}

class Report implements ReportInterface {

  private final Repository rep; 

  Report(Repository ref){
     this.rep = ref;
  }

  public void execute(){
     //do some logic
  }
}


class ReportWithSetter implements ReportInterface {

  private final Repository rep;
  private String release;

  ReportWithSetter(Repository ref){
     rep = ref;
  }

  public void execute(){
     if (release == null) throw IlligalArgumentException("release is not specified");
     //do some logic
  }
  
  public void setRelease(String release){
     this.release=release;
  }
}

The second report needs an additional parameter release to work properly, but my interface is defined without parameters for execute method, so I work around it with a setter method, so it would look like:

ReportWithSetter rep2 = new ReportWithSetter (rep);
rep.setRelease("R1.1");
rep.execute();

So I don't like this additional rep.setRelease. I looks weird and artificial - a user of this class may be confused, and for example, if I make the class as a singleton bean in Spring, it is a source of potential error, if it is requested for the second time and somebody forgets to trigger rep.setRelease for the second time. Besides putting it into constructor (I want to make it a spring bean), what would be the best practice to handling this situation?

If that's the case, then maybe `ReportWithSetter` and `Report` shouldn't implement the same `Report` interface. `ReportWithSetter.execute` should take a `release` parameter. — Sweeper, Oct 12 '20 at 05:09
I would add a `Versionnable` interface with a single `getRelease()`, rename `ReportWithSetter` to `VersionnableReport`, then let it implement both `Report` and `Versionnable`. — Vincent C., Oct 12 '20 at 05:18
What's the problem if you put it into constructor? It's does prevent you from making it a spring bean. — haoyu wang, Oct 12 '20 at 06:20
A sensible solution will be adding a new overloaded method `execute(String release)` to `ReportInterface`, and provide a default value for `execute()` method. — Jerry Chin, Oct 12 '20 at 06:42

score 5 · Accepted Answer · answered Oct 12 '20 at 06:49

Assuming you are allowed to change the interface, here are a few solutions I can think of:

Solution #1

void execute(Optional<String> release);

or

void execute(@Nullable String release);

and then use them for Report class as execute(Optional.empty()) or execute(null).

Solution #2

void execute(String... release);

and then use it for Report class as execute() and for ReportWithSetter class as execute("R1.1").

Solution #3

Define both void execute(); and void execute(String release); in the interface. Then while implementing, throw UnsupportedOperationException in the method you don't need. For example, in Report class, you would do:

  public void execute(){
     //do some logic
  }

  public void execute(String release){
     throw new UnsupportedOperationException("Use the overloaded method");
  }

You can also make both these methods as default in the interface, so your implementation classes don't have to worry about implementing the unsupported method.

Use whichever is most readable and maintainable for you.

The second solution is very iffy, because there is no indication that he needs more than one String on input and it just tries to bend the interface for the sake of usage. +1 though for the other solutions. — Dropout, Oct 12 '20 at 06:54

Elmar Brauch · Answer 2 · 2020-10-12T07:31:30.380

Solution 1: Spring Dependency Injection - Field Injection:

Spring's Dependency Injection works with reflection, so Setter methods are not required.
So if you make your Report class a Spring Bean and use @Autowired to inject another bean, then the Setter method is not required.
It would look like this:

@Component
class ReportWithRelease implements ReportInterface {

@Autowired private final Repository rep;
@Autowired private Release release;

public void execute(){
  if (release == null) throw IlligalArgumentException("release is not specified");
    //do some logic
  }
}

I changed "String release" to "Release release", because making a bean of "String" would be also strange. So the "Release" class would have to contain your "String release".

If "String release" contains only some configured value, which does not change at runtime. Then you can use @Value to read its String value from a properties file.

Solution 2: Spring Constructor Injection:

Constructor injection is another option, which is even more recommended. Then your Report bean would look like this:

@Component
class ReportWithRelease implements ReportInterface {

private Repository rep;
private Release release;

@Autowired
public ReportWithRelease(Repository rep, Release release) {
  this.rep = rep;
  this.release = release;
}

public void execute(){
  if (release == null) throw IlligalArgumentException("release is not specified");
    //do some logic
  }
}

Field injection is really shitty to test and even setter injection is more of a compromise because required dependencies might not exist and you get a nice NPE during runtime. Try to use constructor injection whenever possible! — Christoph Grimmer, Oct 12 '20 at 07:13
@ChristophGrimmer-Dietrich you are right, Constructor Injection is more recommended, so I updated my answer. I don't agree with Field injection being "shitty", but that is out of question's scope. — Elmar Brauch, Oct 12 '20 at 07:25
It's really hard to properly unit test because you either have public non-final fields, protected fields and test with a subclass, or use Mockito injection which is to much reflection magic for my personal liking. Hence "shitty to test" :-) — Christoph Grimmer, Oct 12 '20 at 08:31

score 0 · Answer 3 · answered Oct 12 '20 at 07:01

Factory method patterns are good if you want to create instances of different classes of same interface.

class MyFactory {
       ReportInterface createInstance(Class clazz, String... args) {
           if (Report.class.equals(clazz)) {
               return new Report();
           }
           if (ReportWithSetter.class.equals(clazz)) {
               return new ReportWithSetter(args[0]);
           }
           throw new IllegalArgumentException(clazz.getName());
       }
}

score 0 · Answer 4 · answered Oct 12 '20 at 07:14

Spring of course offers autowiring, but introducing @AutoWire should be done for systematic purposes.

Here you can do with a two-stage execute, a factory.

class ReportFactory /*ReportWithSetter*/ {

  private final Repository rep;
  private final String release;

  private final ReportInterface report = ...;

  ReportFactory (Repository rep, String release) {
     this.rep = rep;
     this.release = release;
  }

  public ReportInterface report() {
      return report;
  }
}

new ReportFactory(rep, release).execute();

Java - Getter/Setter, behavior and Interfaces

4 Answers4