Rule S00107 "Methods should not have too many parameters" should consider Lombok annotations

java
lombok

(Thomas Turrell Croft) #1

The following code smell would be detected by S00107:

public class Application {

  private final String param1;
  private final String param2;
  private final String param3;
  private final String param4;
  private final String param5;
  private final String param6;
  private final String param7;
  private final String param8;
  private final String param9;

  public Application(String param1, String param2, String param3, String param4, String param5, String param6, String param7, String param8, String param9) {
    this.param1 = param1;
    this.param2 = param2;
    this.param3 = param3;
    this.param4 = param4;
    this.param5 = param5;
    this.param6 = param6;
    this.param7 = param7;
    this.param8 = param8;
    this.param9 = param9;
  }

However this code would pass rule S00107:

@RequiredArgsConstructor
public class Application {

  private final String param1;
  private final String param2;
  private final String param3;
  private final String param4;
  private final String param5;
  private final String param6;
  private final String param7;
  private final String param8;
  private final String param9;

The code is identical, both examples have too many parameters however since the second example uses the Lombok RequiredArgs constructor it is not detected. The same is true for the Lombok AllArgsConstructor annotation.


(Nicolas Peru) #2

Hi,
Both codes are indeed equivalent in semantic but not in syntax. And this rule detects a syntax that is avoidable on purpose by using lombok. I tend to believe that raising an issue in such cases with Lombok would be counter productive and defeat the very reason why people use Lombok.

I tend to think that you are maybe more looking for a “Class with too many field rule” ?


(Thomas Turrell Croft) #3

I think (but I’m not sure) that I am looking for a “constructor with too many parameters rule”.

Lombok can be easily be used to hide bad design. I consider a constructor with more than five parameters a poor design regardless of the syntax. This is especially true when using constructor injection.

I haven’t considered the maximum number of fields a class could have. My unconsidered opinion is that a class could have an unlimited number of fields.

It might be true to state that a class could have a maximum number of final fields because final fields can only be set by a constructor. It might also be true to state that a class could have a unlimited number of static final fields.

However I would welcome other peoples thoughts on the matter.


(Vmurthy) #4

Hi,

While still focussing on constructor having too many parameters as an issue couldnt @Builder on methods serve you better. Builder is suggested as a solution for objects having many final fields(and thus required to be parameters in constructor) even in Effective Java and other best design practices.

Whereas if weight of the class (in terms of fields/methods) is the focus then class refactoring is a need.

not sure which one is the focus here. please clarify

thanks
venkat


(Thomas Turrell Croft) #5

I think the builder pattern is useful. However I believe that rule S00107 is very helpful in detecting services which have too many dependancies. If a service has too many dependancies it is probably doing too much.

Before Spring 4.3 the Autowired annotation was required to inject dependancies.

Rule S00107 would detect that the following example has too many dependancies.

@Service
public class FooService {

  private final ServiceA a;
  private final ServiceB b;
  private final ServiceC c;
  private final ServiceD d;
  private final ServiceE e;
  private final ServiceF f;

  @Autowired
  public FooService(ServiceA a, ServiceB b, ServiceC c, ServiceD d, ServiceE e, ServiceF f) {
    this.a = a;
    this.b = b;
    this.c = c;
    this.d = d;
    this.e = e;
    this.f = f;
  }

However with Spring 4.3 and later the Autowired annotation is no longer required meaning that it is now more practicable to use Lombok’s RequiredArgsConstructor annotation. So the following is now possible and rule S00107 will not detect the violation.

@Service
@RequiredArgsConstructor
public class FooService {

  private final ServiceA a;
  private final ServiceB b;
  private final ServiceC c;
  private final ServiceD d;
  private final ServiceE e;
  private final ServiceF f;
 }

I have seen services in pull requests that have too many dependancies. Ironically refactoring the code to use the RequiredArgsConstructor can be used to avoid rule S00107.