Code Smells 1 Null

Null Pointer Exceptions are the most often the thing OOP developer will see while their app sinks. They don’t get much help, all they tell you is that something is not there. Where you might ask and all you get is a line. In this line there are several possibilities to be null. So no real help here. Some people came up with the idea to have less information on a single line, so that the NPE can easier be identified. This is more or less a fix on the result but does not solve the root cause.

In this article I take a look at some of the root cause and how to prevent them.

These example are from the reviews I got through and part of a series of things I find in reviews. The solution and the problem itself might are my own opinions. So yours might be different.

Photo by Dawn Armfield on Unsplash

A service call might return null

Here is an example where a call to a service might return null. How do I know that? You should always check the implementation! Sad thing to do. You should trust nobody!

So every method that returns an Object, might return null.

1. Start

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13

public class StuffController {
  public ResponseEntity<StuffModel> getStuff(@RequestBody StuffInfoModel stuffInfo) {
    var getheredStuff = stuffService.getherSomeStuff(stuffInfo);

    if (getheredStuff != null) {
      LOG.info("Stuff is null!");
    } else {
      LOG.info("Stuff gethered!");
    }

    return ResponseEntity.status(getheredStuff != null ? HttpStatus.OK : HttpStatus.BAD_REQUEST).body(getheredStuff);
  }
}

2. improve returned value

One thing I use often is to put the result of the call where an object is returned into an Optional.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11

public class StuffController {
  public ResponseEntity<StuffModel> getStuff(@RequestBody StuffInfoModel stuffInfo) {
    var getheredStuff = Optional.ofNullable(stuffService.getherSomeStuff(stuffInfo))
      .ifPresentOrElse(
        __ -> LOG.info("Stuff gethered!"),
        () -> LOG.info("Stuff is null!")
      );

    return ResponseEntity.status(getheredStuff != null ? HttpStatus.OK : HttpStatus.BAD_REQUEST).body(getheredStuff);
  }
}

Ok, thats a little bit better, but we can do even better.

1
2
3
4
5

public class StuccController {
  public ResponseEntity<StuffModel> getStuff(@RequestBody StuffInfoModel stuffInfo) {
    return ResponseEntity.of(Optional.ofNullable(stuffService.getherSomeStuff(stuffInfo)));
  }
}

Here I skipped the logging. It is of very little use anyway.

3. moving the Optional up

The last step would be if possible to move the creation of the nullable.

1
2
3
4
5

public class StuffController {
  public ResponseEntity<StuffModel> getStuff(@RequestBody StuffInfoModel stuffInfo) {
    return ResponseEntity.of(stuffService.getherSomeStuff(stuffInfo));
  }
}

You might say, “Yes, but where is the logging?”

I believe logging should only be placed here if the service is part of a 3rd party library. Here I don’t have any control over the return type or anything. But in my own implementation I would use a log aspect to capture the logging for unexpected cases. I’ll write a separate post about logging here, so stay tuned.

Null from a service

In the example above we saw, that a null check was needed because the caller service might give us null back. The service might look like this:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11

public class StuffService {
  public StuffModel getherSomeStuff(StuffInfoModel stuffInfo) {
    StuffModel stuff = null;
    try {
      stuff = StuffRepository.fetch(stuffInfo);
    } catch(Exception ex) {
      LOG.error("Did not went well: {}", ex);
    }
    return stuff;
  }
}

Here if the call is not successful the result is null.

1
2
3
4
5
6
7

public class StuffController {
  public StuffModel getherSomeStuff(StuffInfoModel stuffInfo) {
    Optional<StuffModel> stuff = StuffRepository.fetch(stuffInfo);

    return stuff.orElse(null);
  }
}

With the support of return type Optional<> from the JPA repository functions, we can just keep the line going from end to end.
This should be changed to the following:

1
2
3

public interface StuffRepository extends JpaRepository<> {
  public Optional<StuffModel> getherSomeStuff(StuffInfoModel stuffInfo);
}

and then:

1
2
3
4
5

public class StuffService {
  public StuffModel getherSomeStuff(StuffInfoModel stuffInfo) {
    return stuffrepositiory.fetch(stuffInfo);
  }
}

1
2
3
4
5

public class StuffController {
  public ResponseEntity<StuffModel> getStuff(@RequestBody StuffInfoModel stuffInfo) {
    return ResponseEntity.of(stuffService.getherSomeStuff(stuffInfo));
  }
}

Basically this is all.

Left outs

To keep this example simple I omitted some mapping strategy for Entities, DTOs and Models. This would be chained to the corresponding calls to each component if needed. I also removed log calls. Those are most of the time cluttering inside a code base and increases the level of bad reading experience. From my of view, logging belongs most of the time not to the business model and should be handled behind the scenes by some abstraction. That would create a level of consistency and reduces the overall complexity for developing. Logging will be everywhere and should be as much as possible convenient.

Conclusion

We saw that we reduced the code to a bare minimum by using convenient behavior of frameworks and implicit programming. When some new will get to this code, she/he will have less problems to directly understand what is going on. But the best thing is, by using an implicit proven solution for behavior we are not creating less bugs.