One Line of Code that Cost Me An HOUR to Fix

Hello developers. In today’s article, I am going to share story of how one line of code cost me an hour to find and fix. Let’s get started.

The project was developed in Spring Boot 2.6.4 initially, right now it is operating on Spring Boot 3.2.3 version. I upgraded the version to Spring Boot 3.2.3 because I’ve experienced some issues which blocks and delays the upcoming feature. I will talk about the upgrade journey in another article. Make sure to follow to get notified about upcoming articles.

The Problem

I started my SE journey in a new company and joined an interesting project. The project contains multiple schedulers and each of them runs in different times such as every 10 seconds, 30 seconds, 30 minutes and so on. If you have an experienced in such kind of the project, then you’ve already know how hard it is to detect problems.

Okay let’s go straight to the line that cost me hours of debugging.

public void sendChatRequest(Long chatId) {
   chatService.findChatByName("demo-chat").orElse(save(new Chat("demo-chat")));
   // other codes ommited...
}

Can you spot the problem?

That save() method will be called every time regardless of existence of Chat. This is because orElse() method evaluated eagerly. On the other hand orElseGet() method is evaluated lazily and it will be executed only when Optional is empty.

In order to understand difference between them, execute following code in your IDE and look at logs.

First, we are going to return empty Optional to see what the results will be.

public class OrElseAndOrElseGetMethods {

  record Chat(String name) {
  }

  class ChatService {
    public Optional<Chat> findChatByName(String name) {
      return Optional.empty();
    }
  }

  private final ChatService chatService = new ChatService();

  public static void main(String[] args) {
    var main = new OrElseAndOrElseGetMethods();
    main.testOrElse();
    System.out.println("*************************");
    main.testOrElseGet();
  }

  public void testOrElse() {
    System.out.println("Testing orElse() method");
    chatService.findChatByName("demo-chat")
        .orElse(save(new Chat("demo-chat")));
  }

  public void testOrElseGet() {
    System.out.println("Testing orElseGet() method");
    chatService.findChatByName("demo-chat")
        .orElseGet(() -> save(new Chat("demo-chat")));
  }

  public Chat save(Chat chat) {
    System.out.println("Saving " + chat);
    // saving Chat to database
    return chat;
  }
}

Output of above code snippet is represented below:

Testing orElse() method
Saving Chat[name=demo-chat]
**********
Testing orElseGet() method
Saving Chat[name=demo-chat]

As you can see on the above in both methods (orElse() and orElseGet()) save() method is executed.

Let’s see what will happen if Optional is not empty.

public class OrElseAndOrElseGetMethods {

  record Chat(String name) {
  }

  class ChatService {
    public Optional<Chat> findChatByName(String name) {
      // searching inside db
      var foundedChat = new Chat(name);
      return Optional.of(foundedChat);
    }
  }

  private final ChatService chatService = new ChatService();

  public static void main(String[] args) {
    var main = new OrElseAndOrElseGetMethods();
    main.testOrElse();
    System.out.println("**********");
    main.testOrElseGet();
  }

  public void testOrElse() {
    System.out.println("Testing orElse() method");
    chatService.findChatByName("demo-chat")
        .orElse(save(new Chat("demo-chat")));
  }

  public void testOrElseGet() {
    System.out.println("Testing orElseGet() method");
    chatService.findChatByName("demo-chat")
        .orElseGet(() -> save(new Chat("demo-chat")));
  }

  public Chat save(Chat chat) {
    System.out.println("Saving " + chat);
    // saving Chat to database
    return chat;
  }
}

Whole code snippet on the above is same as before with only difference on line 11. Instead of returning Optional.empty(), Optional.of(foundedChat) is returned.

Let’s see the results

Testing orElse() method
Saving Chat[name=demo-chat]
**********
Testing orElseGet() method

As you can see on the output, save() method is called when executing orElse() method, however, this didn’t happen on orElseGet() method.

Why It Took so Much Time to Detect and Fix the Issue?

I didn’t see some contingencies, but after 20 minutes later I realized, I have more chat request on the database than expected. It was the test environment and my teammates were working on the server at that time. But they tell me they didn’t touch anything on the server at that day.

Debugging session began and two things made me struggle.

There were too many schedulers.
Lack of logging especially on methods where changes happens.
In order to fix this issue, I changed method from orElse() to orElseGet().

Improvements on the Project after That Issue

The first improvement was reducing schedulers count from five to one. I observed and saw that those schedulers were pretty much served the same purpose, so it would be okay to combine them into one scheduler operation. In addition, one of them was outdated and removed immediately.

The second improvement was the addition of self-explanatory logging statements to methods to facilitate debugging process.

Lessons Learned After that Issue

When source of the problem found, I realized I’d looked at this class twice but couldn’t find the first time. This is because I was in rush and tried to fix the problem as fast as possible. However, that cost me more time to figure out where the problem was.

Next time when you encounter this kind of problem, read every line out loud or murmur to yourself, that way code will become much more clear for you.

When you start to apply above strategy into your working, you will find lots of problems or poorly written code. This will give you an advantage to find the places where refactor is needed.

Here’s the example for the above:

- You: Okay, in the first line of the method, it do null checks
- You: if it is null, then return immediately, otherwise
- You: send the method parameter to method findByName()
- You: and findByName() returns Optional. 

You understand the idea. When you do above, don’t be in rush, otherwise this won’t have so much impact. Be patient and slow.

This is the end of the article, if you like the article don’t forget to share and write comments.

If you have any questions, you can reach me via LinkedIn.

At last, code is available on GitHub.