4

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.
3
  • 11
    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. Commented Jun 29, 2022 at 15:54
  • 2
    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. Commented Jun 29, 2022 at 18:12
  • Better than ignoring warnings is to figure out how to change the rules, and carefully change them to only remove the warnings you want to remove. Because you want no warnings, so that “real” warnings stand out, and you don’t want to hide correct warnings.
    – gnasher729
    Commented 12 hours ago

6 Answers 6

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. Commented Jun 29, 2022 at 12:56
  • Especially if your GUI could be English, French, German, Italian, whatever.
    – gnasher729
    Commented Jun 29, 2022 at 13:23
2

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
    Commented Jun 30, 2022 at 12:03
2

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);
        }
    
}
1

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

1

I will certainly complain same as Sonarlint. Coding is teamwork and messy code = more trouble for others. Convention is not enforced, but at least it should be respected whenever possible. Maintaining legacy code is hard due to reasons such as this. Here one confusing thing at first glance and there another, messy messy pile of shit is not made in one day.

I have enums that don't respect the convention to have all uppercase constants

-> 100% dev's fault, very probably not yours work, someone has left code smell for you to fix.

...because when we move to frontend the data there has to be camelCase (or even random / weird case)

-> Totally valid requirement, which serialization libraries can handle/should handle in backend; backend devs needs to find a solution by using the library well.

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

-> Totally valid reason, meanwhile if you care enough, you should still try to find a way if time allows. And communicate with the guy who has done this to the team, if he/she hasn't left your company yet.

Now, for Jackson. It uses @JsonProperty to find out the property names to serialize. I remember it does not affect deserialization but correct me if wrong. It can be put on a getter so you could define a getter as you want. Or a custom converter. Read documentation and decide. If a library cannot handle such a case after reading doc then it should not be used in the first place; but as far as I know, Jackson is quite able.

New contributor
WesternGun is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
0

The answer is straightforward. If the enum is a part of business logic, it should follow Java convention. If enum is part of serialization protocol, it should follow the serialization convention.

Business logic should NEVER depend on serialization. Suggestions to ignore the smell fail to interpret the intent and context. The problem is not that certain names have a improper case. The problem is that serialization protocol has started to affect business logic.

Sonar can not distinguish your domain model and persistence layer, so you have to take the reins here.

Your Answer

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

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