-1

I use magic numbers in class member initialization:

private static final RangeMap<Integer, String> MY_MAP = new ImmutableRangeMap.Builder<Integer, String>()
        .put(Range.closed(10, 31), "random string 1")
        .put(Range.closed(32, 36), "random string 2")
        .put(Range.closed(37, 39), "random string 3")
        .put(Range.closed(40, 78), "random string 4")
        // lot of similar lines here
        .build();

The code is quite clear and easy to read but checkstyle gives us tons of warnings regarding Checkstyle. I can suppress the warning, but I am looking for better solution in Java.

One way is to use constanst and the code would look like this:

private static final Integer RANDOM_STRING_1_START = 10;
private static final Integer RANDOM_STRING_2_START = 32;
private static final Integer RANDOM_STRING_3_START = 37;
private static final Integer RANDOM_STRING_4_START = 40;
private static final Integer RANDOM_STRING_1_END = 31;
private static final Integer RANDOM_STRING_2_END = 36;
private static final Integer RANDOM_STRING_3_END = 39;
private static final Integer RANDOM_STRING_4_END = 78;

private static final RangeMap<Integer, String> MY_MAP = new ImmutableRangeMap.Builder<Integer, String>()
        .put(Range.closed(RANDOM_STRING_1_START, RANDOM_STRING_1_END), "random string 1")
        .put(Range.closed(RANDOM_STRING_2_START, RANDOM_STRING_2_END), "random string 2")
        .put(Range.closed(RANDOM_STRING_3_START, RANDOM_STRING_3_END), "random string 3")
        .put(Range.closed(RANDOM_STRING_4_START, RANDOM_STRING_4_END), "random string 4")
        // lot of similar lines here
        .build();

and I don't think it is as nice and readable as before. Moreover it's more prone to error while typing.

Any ideas how to define table of values without checkstyle complaining? I don't want to suppress the warning, I'd like to find the "Java way".

Michal Špondr
  • 1,337
  • 2
  • 21
  • 44
  • 2
    You should suppress the warning. The first way is _far_ better (or, perhaps, the second way is awful: you shouldn't let tools make you do awful things just to shut them up). – Andy Turner Feb 03 '22 at 15:36
  • Depends: do you use RANDOM_STRING_1_START in more than one part of your code or is a significant number that might be changed later? – dcolazin Feb 03 '22 at 16:05
  • @dcolazin It is not completely random. Let's say it's a string that describes product within a range of serial numbers. – Michal Špondr Feb 04 '22 at 07:41
  • then if it should be consistent (with a significant name), I would use constants (defined in a suitable container class) and not literals: what if you have to change the values in two years? will you remember every place? – dcolazin Feb 04 '22 at 07:44

1 Answers1

0

As stated in the comments, the rule you mentioned does not make sense HERE. Think of it as a helper/reminder: Imagine you have to use a timeout twice or thrice in a class and assign the value "60" to various places. It helps keeping those numbers in sync.

For your case, here are a few things you can try.

General filename exclusion

If it is a class which exists only once, you can use this exclusion in your checkstyle config:

  <module name="BeforeExecutionExclusionFileFilter">
    <property name="fileNamePattern" value="MyClass\.java$" />
  </module>

But I'd rather use it for module-info.java only.

SuppressionFilter

This filter is more specific, as it will only suppress a specific check.

  <module name="SuppressionSingleFilter">
    <property name="checks" value="MagicNumber" />
    <property name="files"
              value="(JpaSomethingFactory|MyModelSomethingFactory).java" />
  </module>

Suppress a range of lines using a comment

You can use this snippet to suppress this warning only in the range of a commented section. In your case, I would recommend this approach as it is the easiest to maintain in case a file name changes, or you do not need it anymore.

<!-- allow switching off some checks -->
    <module name="SuppressionCommentFilter">
      <property name="offCommentFormat" value="CHECKSTYLE.OFF\: ([\w\|]+)" />
      <property name="onCommentFormat" value="CHECKSTYLE.ON\: ([\w\|]+)" />
      <property name="checkFormat" value="$1" />
    </module>

Use it like this:

// CHECKSTYLE:OFF: MagicNumber - Specific test class
private static final RangeMap<Integer, String> MY_MAP = new ImmutableRangeMap.Builder<Integer, String>()
        .put(Range.closed(10, 31), "random string 1")
        .put(Range.closed(32, 36), "random string 2")
        .put(Range.closed(37, 39), "random string 3")
        .put(Range.closed(40, 78), "random string 4")
        // lot of similar lines here
        .build();
// CHECKSTYLE:ON: MagicNumber - Specific test class
Benjamin Marwell
  • 1,173
  • 1
  • 13
  • 36