2

In my Java 11 / Spring Boot codebase I have enums that don't respect the convention to have all uppercase constants because when we move to frontend the data there has to be camelCase (or even random / weird case).

I am not designing this from scratch, more dealing with something pre-existing.

Here's an Enum example:

public enum Direction {
    Up,
    Down
}

If I run SonarLint I get "code smell java:S115 - Constant names should comply with a naming convention".

This means that my enum should look like:

public enum Direction {
    UP,
    DOWN
}

This leads to other problems in the code because when I serialize / deserialize data (that I also get from clients) it has different case differently.

I have read this article on Baeldung and implemented a possible solution:

public enum Direction {
    UP("Up"),
    DOWN("Down");

    private final String value;

    Direction(String value) {
        this.value = value;
    }

    public String getValue() {
        return value;
    }

    @Override
    public String toString() {
        return String.valueOf(value);
    }

    public static Direction fromValue(String value) {
        for (Direction b : Direction.values()) {
            if (b.value.equals(value)) {
                return b;
            }
        }
        throw new IllegalArgumentException("Unexpected value '" + value + "'");
    }
}

This leads to issues in my code where instead of using Direction.valueOf(); I should be using Direction.fromValue();.

While this is feasible for code in my control all the libraries that are using valueOf (for example using Jackson?)...
Will get the wrong data?

How can I solve the issue in the cleanest way?

Edit

  1. As per suggestion I changed the Person example to a more suitable one.
2
  • 6
    This is completely backwards. SonarLint should be a tool that's there to serve you, you shouldn't serve SonarLint. Who cares if it has opinions about how the code should look like. Nobody's going to come and look at your codebase and be like "My God, this is terrible! Your enums don't follow SonarLint's generic ruleset!" You should have the right to make decisions for your own project, because, well, it's your project, and you understand the context and the constraitns. If you can configure the rules, just tell SonarLint to shut up, otherwise ignore it. That's the cleanest way. Jun 29, 2022 at 15:54
  • 1
    I agree with Filip. If there is a programmatic reason to violate a coding convention (i.e. enums used in serialization) then you can safely ignore those warnings. Naming conventions in particular are not errors. Even naming conventions are simply a tool to keep code recognizable in the long term. Jun 29, 2022 at 18:12

4 Answers 4

2
  1. Ignore the code smell warning as these are at best suggestions. I would recommend all constants still following the same pattern in code though.
  2. Follow the standard and let the serializer fix the formatting issues.
  3. Follow the standard and let the UI figure out a display issue in the preferred way for however you are handling UI.

Option 2 is probably the best one long term. The first isn't bad if the project is small, but since you mentioned needing different formats it might not be a good solution. The third option can make any write action from the UI a bit more awkward, so it could be problematic.

2
  • Definitely Option 2. Making a code base readable and making a GUI readable are so different from each other that insisting on the same spelling in both worlds is usually a bad idea. Jun 29, 2022 at 12:56
  • Especially if your GUI could be English, French, German, Italian, whatever.
    – gnasher729
    Jun 29, 2022 at 13:23
1

I implemented another possible solution suggested in this other Baledung article and it worked just fine:

public class DirectionConverter implements AttributeConverter<Direction, String> {
     
        @Override
        public String convertToDatabaseColumn(Direction direction) {
            return (direction == null) ? null :  direction.getValue();
        }
    
        @Override
        public Direction convertToEntityAttribute(String direction) {
            log.error("Converting {} to {} ({})", direction, Direction.fromValue(direction), Direction.fromValue(direction).name());
            return (direction == null) ? null : Direction.fromValue(direction);
        }
    
}
0

In addition to Ryathal's answer.

In situations like this, it's reasonable to spend some time getting familiar with our libs and their features. Do some proof of concept, tests, etc. See what can we do with what we have now. Alternatively, consider including another lib although it is not always possible.

If you already included Jackson in your project, try with Custom Serializers and Custom Deserializers, from which you can handle the input, change it to make it suitable for .valueOf(string).1

Ignoring Sonar's warnings is reasonable sometimes. I would use @SuppressWarnings("squid:S115") if I consider this case to be marginal or anecdotal. I prefer to keep the quality gates as strict as possible and decide when an issue is a real issue or not right over the code.


1: See also jackson annotations

0

Java naming conventions make sense inside Java, and generally, they should be followed.

But in your case, you have a given interface, not designed with Java naming conventions in mind. It absolutely makes sense to represent things that are enumerations in your interface as Java enum. Sonar as a dumb automat doesn't know about the external requirement, and complains.

You have two options:

  • You name the enum values after the external interface values. Then you "violate" the Java conventions, but have a simple, straightforward solution with low failure risk.
  • You name the enum values according to the Java conventions, and add code and/or configuration for the translation between the two naming schemes. That adds complexity and failure risk to your solution.

Sonar as a dumb automat doesn't know about the external requirement, only about the naming conventions, and complains.

When evaluating the two options, I'd clearly go for the first one, add a documentation to the enum stating that its naming is based on a given external requirement, and suppress the Sonar warning.

A remark on your example: I hope you don't really have a Person enum which can be either Name or Surname, meaning that there are only two persons in the world:

  • Name is one Person,
  • Surname is the only other existing Person.

Such a Person class name would be a horrible mis-nomer. An enum with these two values should better be called NamingType, PersonFieldName or similar.

1
  • Very good point, Ralf. It was just a very poor example jotted in... Too short time! :)
    – Pitto
    Jun 30, 2022 at 12:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.