Are AI Coding tools worth it – Part 2

In Part 1, I started to examine the value of AI tools for developers, especially those that help them create or understand code. The tools selected are all free to use and are a mix of general-purpose ones and those that declare they are specifically for coding purposes

Tools being evaluated

1: ChatGPT  https://chatgpt.com/

2: Claude https://claude.ai/

3: Amazon Q Developer  https://aws.amazon.com/q/developer/

4: Codeium https://codeium.com/

5: JetBrains AI Assistant https://www.jetbrains.com/ai/

I asked each tool three questions

1. “Generate the main Javadoc to describe this class.”
2. “Assess the class for bugs and coding weaknesses and provide corrections or guidance.”
3. ” Generate Junit test cases to provide around 90% code coverage -especially for edge conditions etc.”

Results so FAR

In Part 1, we saw that they could all generate Javadoc, but none could do it particularly well. ChatGPT came off best as providing the most comprehensive response, but all the tools were prone to hallucinations that made the result misleading.

Read on for the next part, which is assessing the code for bugs and weaknesses.

Code Smells and other problems

Here’s the primarily verbatim output from each tool. I know that many of you will be shouting, “Show me the code,”   but that’s for later. Right now, I’m comparing the similarity between tools’ responses. We’ll talk about accuracy later.

I’ll repeat that almost 99% of the following , right up until the summary, are words and code taken straight from the tools.

Codium

Magic numbers: The code uses magic numbers (e.g., 0x0001, 0x0200, 0x4000) in the isPublic(), isInterface(), and isEnum() methods. It's better to define these numbers as constants with descriptive names to improve readability and maintainability.

Method naming: Some method names, such as isFinal(), are not very descriptive. Consider renaming them to better reflect their purpose.

Method organization: The class seems to have a mix of methods that operate on the class file itself and methods that operate on its attributes, fields, and methods. Consider grouping related methods together or breaking them out into separate classes to improve organization and cohesion.

Attribute handling: The attributes() method returns an AttributeSet object, but it's not clear how attributes are added or removed from this set. Consider adding methods for attribute management to make the class more usable.

Field and method handling: The fields() and methods() methods return FieldSet and MethodSet objects, respectively. However, it's not clear how fields and methods are added or removed from these sets. Consider adding methods for field and method management to make the class more usable.

Constant pool handling: The constantPool object is used in several methods, but its purpose and behavior are not clear. Consider adding documentation or comments to explain its role in the class.

Error handling: The class does not seem to handle errors or exceptions explicitly. Consider adding try-catch blocks or throwing exceptions to handle potential errors and make the class more robust.

Code formatting: The code formatting is mostly consistent, but there are some minor issues (e.g., inconsistent indentation, missing whitespace). Consider running a code formatter to improve the overall appearance of the code.

Amazon q Developer

This one was a little strange. I had the following conversation with the chatbot..

After which we go the following advice

The class seems to be defining an enumeration Type with values CLASS, INTERFACE, ENUM, and UNKOWN (which appears to be misspelled and should be UNKNOWN).

There's a method isClass() that checks if the instance is not an enum and not an interface. This logic implies that the class might be using flags or other methods to determine its type.

The isPublic() method uses a bitwise AND operation to check if the ACCESS_PUBLIC flag (0x0001) is set in the access_flags field.

The type() method seems incomplete in the visible part of the code. It only handles the case when isClass() is true.

The code follows Java naming conventions for methods and classes.

There might be a potential issue with the toString() method, as it directly returns the result of name() without any null checks.

JetBrains aI assistant

1: Typo in Enum Type:

public enum Type { CLASS,INTERFACE,ENUM , UNKOWN}

The word UNKOWN should likely be UNKNOWN.

2: Inefficient toString() Implementation:

public String toString() { return name(); };

The semicolon at the end of this line is unnecessary and should be removed for consistency.

3: Potentially Unnecessary Implementation Shortness:

Many method implementations are omitted for brevity (/* implementation omitted for shortness */). Ensure that the actual implementations handle edge cases and input validations properly.

4: Map Initialization in MethodSet:

private Map<String,Method> bySignature =new TreeMap<>();

Depending on the actual use case, consider if a TreeMap is necessary. If insertion order preservation is more critical than key ordering, a LinkedHashMap could be more appropriate.

5: Private Constructor without Explanation:

private MethodSet(Method[] methods) {/* implementation omitted for shortness */}

6: Stream Method Without Body:

public Stream<Method> stream() {/* implementation omitted for shortness */}

Ensure that the stream implementation correctly handles cases like empty method sets or large data sizes.

7: Potential Incomplete Exception Handling:

When dealing with class files and streams, it's crucial to handle potential I/O exceptions meticulously. Ensure that file reading operations (even though not shown) handle all possible error states.

8: Mutability Concerns:

Check if all the member variables of the class and its nested classes are properly encapsulated. Consider making fields final where applicable to enforce immutability.

9: Use of Magic Numbers:

The code snippet references a major variable directly without context. Ensure the use of such magic numbers is well-documented, and consider using descriptive constants.

10: Documentation and Comments:

While there are some comments and a detailed class-level Javadoc, ensure that all public methods and crucial implementations are thoroughly documented to make maintenance easier.

Claude.io

1: Inconsistent access modifiers:

Many fields and methods have default (package-private) access, which might not be intentional and could lead to unexpected access from other classes in the same package.

2: Potential null pointer exceptions:

In several methods, there are no null checks before accessing object properties. For example, in the getUTF8AsString method, If getUTF8(idx) returns null, this will throw a NullPointerException.

public String getUTF8AsString(int idx) {return new String(getUTF8(idx));}

3: Unused fields and methods:

There are several fields and methods that are declared but never used within the provided code, such as extraneousData, isProtected() for fields, and digest. These should either be removed or used appropriately.

4: Inconsistent naming conventions:

Some variables use snake_case (e.g., access_flags, this_class) while others use camelCase. It’s better to stick to Java naming conventions and use camelCase consistently.

5: Potential integer overflow:

In the javaVersion() method, there’s no check for potential integer overflow when converting the major version to a string representation.

6: Lack of input validation:

Many methods don’t validate their input parameters. For example, the ConstantPool class methods don’t check if the provided indices are within the bounds of the table array.

7: Mutable public fields:

The Attribute class has public fields, which violates encapsulation and could lead to unexpected state changes.

8: Inefficient use of exceptions for control flow:

In the checkType method, exceptions are used for control flow, which can be inefficient:

private void checkType(int idx,Object[] data, ConstantPoolEntry expected) throws ParseException {
    ConstantPoolEntry type= (ConstantPoolEntry) data[0];
    if(type!=expected) throw new ParseException("constant pool entry "+idx+" expected "+expected+" found "+type);
}

9: Lack of synchronization:

If this class is intended to be used in a multi-threaded environment, there’s no synchronization mechanism in place, which could lead to race conditions.

10: Inconsistent error handling:

Some methods throw runtime exceptions, others throw checked exceptions, and some don’t handle errors at all. A more consistent approach to error handling would improve the robustness of the code.

11: Memory inefficiency:

The ConstantPool class uses an array of Object arrays, which could be memory inefficient for large class files. Consider using more memory-efficient data structures.

12: Potential for resource leaks:

There are no explicit resource management techniques used (such as try-with-resources) which could lead to resource leaks if not handled properly by the calling code.

13: Use of raw types:

The code uses raw types in some places (e.g., Map instead of Map<K,V>), which is discouraged in modern Java.

14: Lack of documentation:

Many methods and classes lack proper JavaDoc comments, which makes the code harder to understand and maintain.

ChatGPT

1. Typo in Enum Declaration

The enum Type has a typo for the value UNKOWN. It should be UNKNOWN

public enum Type { CLASS, INTERFACE, ENUM, UNKNOWN }

2: Inconsistent Method Formatting

The toString() method has an unnecessary semicolon after the method body.

public String toString() { return name(); }

3: Null Pointer Handling

Ensure that all fields like constantPool, this_class, methods, attributes, etc., are properly initialized or checked for null before accessing them to avoid potential NullPointerException (NPE). For example:

public String name() {
    if (constantPool == null || this_class == null) {
       return "Unknown Class";
     }
   return constantPool.getResolvedClassName(this_class);
}

4: Redundant Code in type() Method

The type() method currently checks for each type (CLASS, ENUM, INTERFACE). Since the method isClass() already covers the cases of it not being an enum or an interface, the type() method can be simplified. This eliminates the redundant checks and makes the code easier to follow.

public Type type() {
    if (isEnum()) return Type.ENUM;
    if (isInterface()) return Type.INTERFACE;
    return Type.CLASS; // defaulting to CLASS if it's neither an enum nor an interface
}

5: Avoiding Potential NPEs in Map Operations

In the MethodSet and FieldSet classes, when getting entries from the maps, there should be explicit null checks or default behavior defined. This avoids null references and allows more control over the behavior when a method or field is not found.

public Method bySignature(String d) {
    return bySignature.getOrDefault(d, null);
}

public Field byName(String name) {
    return byName.getOrDefault(name, null);
}

6: Use of TreeMap vs HashMap

In multiple locations (e.g., MethodSet, FieldSet, AttributeSetImpl), TreeMap is used, which keeps entries sorted. If sorting is not a strict requirement, using HashMap might be more efficient due to lower overhead for lookup operations. If sorting is needed, then leaving TreeMap is fine, but be sure to justify its use in those cases.

private Map bySignature = new HashMap<>(); // Use HashMap for performance

7: Code Duplication in AttributeSetImpl

The logic to check for an attribute is repeated in two methods (hasAttribute(String name) and hasAttribute(HSAttributeName name)). You can eliminate code duplication by calling one method from the other:

@Override
public boolean hasAttribute(HSAttributeName name) {
    return hasAttribute(name.name());
}

8: Potential Performance Optimization in AttributeSetImpl.add()

In the add() method, the code checks if a List exists in the attributeByName map, and if it doesn’t, it creates one. You can use computeIfAbsent() to simplify this. This is cleaner and avoids multiple lookups in the map.

private void add(Attribute attribute) { 
    attributeByName.computeIfAbsent(attribute.name(), k -> new LinkedList<>.         ()).add(attribute);
}

9: Magic Numbers in access_flags

The use of magic numbers such as 0x0001 (for public), 0x0010 (for final) in the isPublic() and isFinal() methods can be replaced by constants to improve readability. This makes it easier to understand what the bitwise operations are doing and makes the code more maintainable.

private static final int ACCESS_PUBLIC = 0x0001;
private static final int ACCESS_FINAL = 0x0010;

public boolean isPublic() {
  return (access_flags & ACCESS_PUBLIC) == ACCESS_PUBLIC;
}

public boolean isFinal() {
  return (access_flags & ACCESS_FINAL) == ACCESS_FINAL;
}

10: Uninitialized Variables

Some fields (e.g., constantPool, this_class, methods, fields) are used without initialization. Ensure they are initialized either in a constructor or before use, and validate them for null where appropriate. For example, using constructor injection:

public HSClassFile(ConstantPool constantPool, int this_class, MethodSet methods, FieldSet fields) {
    this.constantPool = constantPool;
    this.this_class = this_class;
    this.methods = methods;
    this.fields = fields;
}

11: Class Accessibility

Ensure that inner classes like MethodSet, FieldSet, and AttributeSetImpl are correctly encapsulated if they are only meant to be used within the HSClassFile class. You may want to make them private if they are internal helpers:

private static final class MethodSet { … }
private static final class FieldSet { … }

12: Missing Documentation

Consider adding Javadoc comments to methods and classes. This is especially important for complex classes that are dealing with low-level concepts (e.g., constant pool, access flags):

/**
 * Returns the name of the class from the constant pool.
 */

public String name() { ... }

Assessing the results

Here’s a summary of the types of results across the tools

• Typo Handling
  • ChatGPT: Identified typo in enum declaration, suggested correction from `UNKOWN` to `UNKNOWN`.
  • Claude: Identified same typo and suggested correction.
  • Amazon Q Developer: Identified the typo in enum declaration.
  • Codeium: Not explicitly mentioned.
  • JetBrains AI Assistant: Detected the typo in the enum declaration and advised correction.
• Magic Numbers
  • ChatGPT: Suggested replacing magic numbers with descriptive constants for readability.
  • Claude: Identified the use of magic numbers but didn’t provide a concrete solution.
  • Amazon Q Developer: Recognized magic numbers but didn’t focus on improvement.
  • Codeium: Highlighted the use of magic numbers and recommended defining constants.
  • JetBrains AI Assistant: Noted magic numbers in method calls and recommended replacing them with constants.
• Null Pointer Handling
  • ChatGPT: Warned about potential `NullPointerException`, advised proper checks before accessing variables.
  • Claude: Highlighted lack of null checks, particularly in the `getUTF8AsString()` method.
  • Amazon Q Developer: Suggested adding null checks in the `toString()` method but did not go into detail.
  • Codeium: No explicit mention of null pointer handling.
  • JetBrains AI Assistant: Identified incomplete null checks and advised more thorough validation.
• Code Duplication
  • ChatGPT: Identified duplicated logic in attribute checking methods and suggested refactoring.
  • Claude: Did not identify or address code duplication.
  • Amazon Q Developer: Did not mention code duplication.
  • Codeium: Did not discuss code duplication.
  • JetBrains AI Assistant: Highlighted duplication in several areas and recommended refactoring to minimize.
• Error Handling
  • ChatGPT: Emphasized consistent error handling across methods and recommended adding missing exception handling.
  • Claude: Identified lack of error handling and inconsistencies.
  • Amazon Q Developer: Suggested adding basic error handling but didn’t provide specific advice.
  • Codeium: Focused on missing exception handling and proposed adding it for robustness.
  • JetBrains AI Assistant: Mentioned incomplete error handling and advised adding detailed exception management strategies.
• Mutability and Encapsulation
  • ChatGPT: Suggested making fields final to improve immutability and encapsulation.
  • Claude: Mentioned mutable public fields that should be encapsulated.
  • Amazon Q Developer: Did not explicitly address mutability concerns.
  • Codeium: Recommended improving encapsulation of fields but did not suggest immutability enhancements.
  • JetBrains AI Assistant: Warned about lack of encapsulation and proposed making fields immutable where possible.
• Performance Optimization
  • ChatGPT: Advised switching from `TreeMap` to `HashMap` for performance when sorting isn’t required.
  • Claude: Did not discuss performance optimization.
  • Amazon Q Developer: Did not mention performance optimization.
  • Codeium: Focused on code readability rather than performance improvement.
  • JetBrains AI Assistant: Noted potential performance hit from using `TreeMap` where `HashMap` would suffice, suggested refactoring.
• Method Organization
  • ChatGPT: No specific suggestion for method organization.
  • Claude: Criticized inconsistent access modifiers and method organization.
  • Amazon Q Developer: Vaguely mentioned issues with method logic but did not give structural advice.
  • Codeium: Highlighted the disorganization of methods and suggested grouping them logically.
  • JetBrains AI Assistant: Recommended improving method grouping, particularly with helper classes.
• Use of Exceptions for Control Flow
  • ChatGPT: Highlighted inefficient use of exceptions for control flow and recommended restructuring.
  • Claude: Warned about using exceptions for control flow and proposed handling via alternative methods.
  • Amazon Q Developer: Did not explicitly mention exception handling issues related to control flow.
  • Codeium: Did not mention the use of exceptions for control flow.
  • JetBrains AI Assistant: Suggested reworking exception usage to avoid control flow reliance, echoed ChatGPT and Claude’s concerns.
• Documentation
  • ChatGPT: Advised improving documentation, particularly for complex methods and low-level code.
  • Claude: Stressed the lack of sufficient documentation across methods.
  • Amazon Q Developer: Did not address documentation issues.
  • Codeium: Proposed improving Javadoc comments for better maintainability.
  • JetBrains AI Assistant: Encouraged better documentation to aid in understanding and maintaining the code.

Summary

The results were not as I would have expected.

For example:

For Typo Handling, it was an easy win for all the tools except Codium. Magic Numbers were spotted by all but only ChatGPT gave concrete examples. Null Pointer Handling was another spotty result. Claude.io and Amazon Q developer called out specific methods while ChatGPT went further and provided concrete examples and explained how to use Map.getOrDefault() method; Codium again said nothing.

ChatGPT and JetBrains AI Assistant were the only ones to offer help with Code Duplication, while all the tools offered some suggestions for error handling.

Closing Thoughts

Overall, I’m disappointed and perplexed. The lack of range or accuracy of the Code AIs and the general capability of the general-purpose AIs to achieve similar or better results suggest that I’m not using these Code AIs correctly or that this is all smoke and mirrors.

I’d certainly be willing to use both Claude.io and ChatGPT as code reviewers and, to an extent, as code or doc generators. I have the others as IntelliJ plugins; occasionally, they give good suggestions for small snippets of code as I type. Often, I get suggestions that are just boilerplate and not contextually aware. IntelliJ’s refactoring support is much more useful to me and offers more day-to-day value.

The winner is still ChatGPT; Claude.io sometimes is a little better, but overall I’d still keep with ChatGPT.

I was really hoping otherwise.

In the next part, we’ll examine test case generation. Will ChatGPT win the race, or will a new contender emerge?