Clean code - Where should @Autowired be applied?

Question

I'll start with a simple example. You have a Spring boot application that runs a CommandLineRunner class on initialization.

// MyCommandLineRunner.java
public class MyCommandLineRunner implements CommandLineRunner {
    private final Log logger = LogFactory.getLog(getClass());
    @Autowired //IntelliJ Warning
    private DataSource ds;
    @Override
    public void run(String... args) throws Exception {
        logger.info("DataSource: " + ds.toString());
    }
}
// Application.java
@SpringBootApplication
public class Application {
    public static void main(String... args) {
        SpringApplication.run(Application.class, args); 
    }
    @Bean
    public MyCommandLineRunner schedulerRunner() {
        return new MyCommandLineRunner();
    }
}

Now, like this, this works, everything is OK. However, IntelliJ reports a warning where @Autowired is located (I marked where in the comment)

Spring team recommends: Always use constructor based dependency injection in your beans. Always use assertions for mandatory dependencies.

Now if I follow this, I have a constructor based dependency injection

@Autowired
public MyCommandLineRunner(DataSource ds) { ... }

This also means that I have to edit Application.java as well, since the constructor needs an argument. In Application.java if I try to use the setter injection, I'll get the same warning. If I refactor that as well, I'll end up with some, in my opinion, nasty code.

// MyCommandLineRunner.java
public class MyCommandLineRunner implements CommandLineRunner {
    private final Log logger = LogFactory.getLog(getClass());
    private DataSource ds;
    @Autowired // Note that this line is practically useless now, since we're getting this value as a parameter from Application.java anyway.
    public MyCommandLineRunner(DataSource ds) { this.ds = ds; }
    @Override
    public void run(String... args) throws Exception {
        logger.info("DataSource: " + ds.toString());
    }
}
// Application.java
@SpringBootApplication
public class Application {
    private DataSource ds;
    @Autowired
    public Application(DataSource ds) { this.ds = ds; }
    public static void main(String... args) {
        SpringApplication.run(Application.class, args); 
    }
    @Bean
    public MyCommandLineRunner schedulerRunner() {
        return new MyCommandLineRunner(ds);
    }
}

The above code yields the same result, but doesn't report any warnings in IntelliJ. I'm confused, how is the 2nd code better than the first one? Am I following an incorrect logic? Should this be wired differently?

In short, what's the correct way to do this?

note DataSource is just a pure example, this question applies to anything being autowired.

note 2 Just to say that MyCommandLineRunner.java can't have another, empty, constructor, since DataSource needs to be autowired/initialized. It will report an error and will not be compiled.

Answer 1

There are several ways to improve it.

1. You can remove @Autowired from your MyCommandLineRunner as you are letting a @Bean method construct an instance of it. Inject the DataSource directly into the method as an argument.

2. Or remove @Autowired and remove the @Bean and slap a @Component annotation on your MyCommandLineRunner to have it detected and remove factory method.

3. Inline your MyCommandLineRunner inside your @Bean method as a lambda.

No Autowiring in the MyCommandLineRunner

public class MyCommandLineRunner implements CommandLineRunner {
    private final Log logger = LogFactory.getLog(getClass());
    private final DataSource ds;

    public MyCommandLineRunner(DataSource ds) { this.ds = ds; }

    @Override
    public void run(String... args) throws Exception {
        logger.info("DataSource: " + ds.toString());
    }
}

And the application class.

@SpringBootApplication
public class Application {

    public static void main(String... args) {
        SpringApplication.run(Application.class, args); 
    }

    @Bean
    public MyCommandLineRunner schedulerRunner(DataSource ds) {
        return new MyCommandLineRunner(ds);
    }
}

Usage of @Component

@Component
public class MyCommandLineRunner implements CommandLineRunner {
    private final Log logger = LogFactory.getLog(getClass());
    private final DataSource ds;

    public MyCommandLineRunner(DataSource ds) { this.ds = ds; }

    @Override
    public void run(String... args) throws Exception {
        logger.info("DataSource: " + ds.toString());
    }
}

And the application class.

@SpringBootApplication
public class Application {

    public static void main(String... args) {
        SpringApplication.run(Application.class, args); 
    }

}

Inline CommandLineRunner

@SpringBootApplication
public class Application {

    private static final Logger logger = LoggerFactory.getLogger(Application.class)

    public static void main(String... args) {
        SpringApplication.run(Application.class, args); 
    }

    @Bean
    public MyCommandLineRunner schedulerRunner(DataSource ds) {
        return (args) -> (logger.info("DataSource: {}", ds); 
    }
}

All of these are valid ways of constructing your instances. Which one to use, use the one that you feel comfortable with. There are more options (all variations on the ones mentioned here).

Answer 2

Consider making the field ds final, then you don't need @Autowired. See more about dependency injection http://docs.spring.io/spring-boot/docs/current/reference/html/using-boot-spring-beans-and-dependency-injection.html#using-boot-spring-beans-and-dependency-injection

To keep the code clean, have you considered using Lombok annotations? @RequiredArgsConstructor(onConstructor = @__(@Autowired)) would generate the constructor with @Autowired annotations. See more here https://projectlombok.org/features/Constructor.html

Your code could look like this:

@Slf4j
@RequiredArgsConstructor
// MyCommandLineRunner.java
public class MyCommandLineRunner implements CommandLineRunner {

    //final fields are included in the constructor generated by Lombok
    private final DataSource ds;

    @Override
    public void run(String... args) throws Exception {
        log.info("DataSource: {} ", ds.toString());
    }
}

// Application.java
@SpringBootApplication
@RequiredArgsConstructor(onConstructor_={@Autowired}) // from JDK 8
// @RequiredArgsConstructor(onConstructor = @__(@Autowired)) // up to JDK 7
public class Application {

    private final Datasource ds;

    public static void main(String... args) {
        SpringApplication.run(Application.class, args);
    }

    @Bean 
    public MyCommandLineRunner schedulerRunner() {
        return new MyCommandLineRunner(ds);
    }
}

Later edit

Solution without Lombok relies on Spring to inject dependency when the bean is created

@SpringBootApplication
public class Application {

    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

    @Bean
    /**
     * dependency ds is injected by Spring
     */
    public MyCommandLineRunner schedulerRunner(DataSource ds) {
        return new MyCommandLineRunner(ds);
    }
}

// MyCommandLineRunner.java
public class MyCommandLineRunner implements CommandLineRunner {
    private final Log logger = LogFactory.getLog(getClass());

    private final DataSource ds;

    public MyCommandLineRunner(DataSource ds){
        this.ds = ds;
    }

    @Override
    public void run(String... args) throws Exception {
        logger.info("DataSource: "+ ds.toString());
    }
}