LLM use for Code review #

The fundamental property of Large Language Models is their ability to understand the context of provided content. A straightforward application of this ability is text summarization, correction and any advanced work with natural language. For software developers, it opens possibilities for an independent check of their code from the contextual point of view. There are plenty of tools already for this kind of task, mostly based on online services and therefore utilizng the powerful and really large language models.

The obvious problem is that in the background your code is sent to the cloud for analysis. And, maybe, using the top level cloud model is quite an overkill. Let us take a look if you can use local LLM for this task, in this case, we use LokAi and its Smarter model (currently, it uses Meta’s Llama-3.1-8B model).

Code sample #

I will use a small JAVA class as a test sample. The code is a real life sample from a large commercial project. As a standalone piece of code it will not compile but that’s not a point now. Moreover, I changed names in it so that it can not be identified but the overall structure and functionality is not changed. Try to review it by yourself. How many problems can you find in it?

package com.lubotorok.aitest.Address;

import com.lubotorok.aitest.Address.StayDto;

public class StayAddress extends Address { 
    StayDto adr = null;
 
    public StayAddress(StayDto adr) { 
        super(adr); 
        this.adr = adr; 
    }

    public String getStayBeginDate() {
        return 
            (this.adr == null || this.adr.getStayBeginDate() == null) ? null  :
                this.adr.getStayBeginDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
    }
 
    public String getStayEndDate() { 
        return 
            (this.adr == null || this.adr.getStayEndDate() == null) ? null : 
                this.adr.getStayBeginDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
    }
    
    public String getStayType() { 
        return adr.getStayType(); 
    } 
}

LokAi prompt #

In this example, we just ask LokAi to return an exactly defined one-line evaluation of the given code, with the review following in the next lines. The first line of the review must consist of one of the following strings: “OK”, “Error”, or “Needs review”. One can ask the model to return a more sophisticated structure, but for now, this is sufficient.

Here is the prompt:

Here is source code of a java class:

package com.lubotorok.aitest.Address;

import com.lubotorok.aitest.Address.StayDto;

public class StayAddress extends Address { 
   StayDto adr = null;

   public StayAddress(StayDto adr) { 
       super(adr); 
       this.adr = adr; 
   }

   public String getStayBeginDate() {
       return 
           (this.adr == null || this.adr.getStayBeginDate() == null) ? null  :
               this.adr.getStayBeginDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
   }

   public String getStayEndDate() { 
       return 
           (this.adr == null || this.adr.getStayEndDate() == null) ? null : 
               this.adr.getStayBeginDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
   }
   
   public String getStayType() { 
       return adr.getStayType(); 
   } 
}

do a code review and find any problems with the code. Suggest any improvements. In the begining of your review start with one of the following term: “OK”, “Error”, “Needs review”. “OK” is for code which does not need any special improvements. “Error” is for code that contains any code which can finish in error in specific situations. “Needs review” is for code which can be improved but does not contain errors. Your answer HAS TO START WITH ONE OF THE “OK”, “Error” or “Needs review” tags. Your review should follow on a new line.

LokAi’s output #

Needs review

The code appears to be well-structured and readable. However, there are a few areas that could be improved for better maintainability and performance.

The getStayBeginDate() and getStayEndDate() methods have a similar pattern, but with a subtle mistake in getStayEndDate(). Instead of calling this.adr.getStayEndDate(), it calls this.adr.getStayBeginDate(). This could lead to incorrect date values being returned.

It would be better to extract a separate method for getting the date value from the StayDto object, to avoid code duplication.

private String getDateValue(Date date) {
    return date == null ? null : date.toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
}

Then, the getStayBeginDate() and getStayEndDate() methods can be simplified to:

public String getStayBeginDate() {
    return getDateValue(this.adr.getStayBeginDate());
}

public String getStayEndDate() {
    return getDateValue(this.adr.getStayEndDate());
}

Additionally, the adr field is not necessary, as it can be replaced with a direct reference to the StayDto object passed to the constructor. This would simplify the code and reduce memory usage.

public class StayAddress extends Address {
    private final StayDto adr;

    public StayAddress(StayDto adr) {
        super(adr);
        this.adr = adr;
    }

    // ...
}

The getStayType() method is straightforward, but it would be good to add a null check for the adr object, to prevent NullPointerException.

public String getStayType() {
    return adr == null ? null : adr.getStayType();
}

Overall, the code is well-structured, but there are opportunities for improvement to make it more maintainable and efficient.

Copilot’s output #

I did a comparison with Copilot with the exactly the same prompt as for LokAi. Here is Copilot’s output:

Needs review

1. Date Formatting Issue:

   • In the getStayEndDate method, you are calling this.adr.getStayBeginDate() instead of this.adr.getStayEndDate(). This will return the begin date instead of the end date.
   • Improvement: Correct the method to use this.adr.getStayEndDate().

   public String getStayEndDate() { 
       return 
           (this.adr == null || this.adr.getStayEndDate() == null) ? null : 
               this.adr.getStayEndDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
   }
   

2. Null Check Redundancy:

   • The null check for this.adr is repeated in both getStayBeginDate and getStayEndDate methods.
   • Improvement: Consider extracting the null check into a private method to reduce redundancy.

   private boolean isAdrNullOrEmpty() {
       return this.adr == null;
   }
   
   public String getStayBeginDate() {
       return isAdrNullOrEmpty() || this.adr.getStayBeginDate() == null ? null :
           this.adr.getStayBeginDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
   }
   
   public String getStayEndDate() { 
       return isAdrNullOrEmpty() || this.adr.getStayEndDate() == null ? null : 
           this.adr.getStayEndDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
   }
   

3. Potential NullPointerException:

   • In the getStayType method, if adr is null, calling adr.getStayType() will throw a NullPointerException.
   • Improvement: Add a null check for adr in the getStayType method.

   public String getStayType() { 
       return adr == null ? null : adr.getStayType(); 
   }
   

4. Code Readability:

   • The code can be made more readable by using meaningful variable names and consistent formatting.
   • Improvement: Rename adr to stayDto for better clarity.

   public class StayAddress extends Address { 
       StayDto stayDto = null;
   
       public StayAddress(StayDto stayDto) { 
           super(stayDto); 
           this.stayDto = stayDto; 
       }
   
       public String getStayBeginDate() {
           return isAdrNullOrEmpty() || this.stayDto.getStayBeginDate() == null ? null :
               this.stayDto.getStayBeginDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
       }
   
       public String getStayEndDate() { 
           return isAdrNullOrEmpty() || this.stayDto.getStayEndDate() == null ? null : 
               this.stayDto.getStayEndDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
       }
   
       public String getStayType() { 
           return stayDto == null ? null : stayDto.getStayType(); 
       }
   }
   

These improvements should enhance the code’s readability, maintainability, and robustness.

Conclusion #

I came up with this piece of code for one reason. It crashed in production after two years of deployment due to an error in this method (did you spot it?):

  public String getStayEndDate() { 
        return 
            (this.adr == null || this.adr.getStayEndDate() == null) ? null : 
                this.adr.getStayBeginDate().toString(DateTimeFormat.forPattern("dd.MM.yyyy"));
  }

It is an obvious CTRL-C, CTRL-V error, but this code will compile and run until the getStayBeginDate() method returns null and then it crashes with a NullPointerException because the other method was tested for returning null value. Moreover, in other cases, it is returning the wrong value. An automatic test would probably catch this problem if written well.

As you can see in the outputs, both models, LokAi and Copilot, provided similar analysis and both of them were able to spot this error. Both suggested the null check in the getStayType() method as well.

All the other review remarks were similar with Copilot being a bit more sophisticated. So I guess we can conclude that for code review, even a small locally running model can be very helpful, especially when privacy is needed.