Make Validation Explicit

Posted on Posted in programming

Does the following method have single responsibility?

public Foo getFooById(String id) {
    Foo foo = fooRepository.findById(id); // gets data
    if (foo == null){ // validate data
        throw new RuntimeException("Foo does not exists.");
    }
    return foo; // return data
}

This method does two different things. It gets data from the repository and it validates returned data.

When we have simple methods, it can not hurt much. If we had a service that got different data from different repositories, code would become intertwined with validation and other logic.

Also if we wanted to have validation without using the exception mechanism, we would have to wrap all return values with the class that can hold error and value.

Example explicit and separated

@Service
public class FooService {

    @Autowired
    FooRepository fooRepository;

    public Foo getFooById(String id) {
        return fooRepository.findById(id);
    }
}
@Service
public class FooValidator {
    @Autowired
    FooRepository fooRepository;

    public Error validate(String id) {
        Foo foo = fooRepository.findById(id);
        if (foo == null){
            return new Error("Foo does not exists.");
        }
        return new Error();
    }
}
In some upper level method:
@RequestMapping("/foo/{id}")
public Foo getById(@PathVariable("id") String id) {
    Error validateResult = fooValidator.validate(id);
    if(validateResult.hasError()){
        throw new RuntimeException(validateResult.getError());
    }
    Foo foo = fooService.getFooById(id);
    return foo;
}

Conclusion

There can be also trade off. When we separate validation and other logic, it could have influence on performance. We make separated call to fetch data, when validation is executed and when other logic is executed.

Leave a Reply

Your email address will not be published. Required fields are marked *