objectcomputing / check-ins

Other
7 stars 7 forks source link

Some sonar cleanup (picking the most prevalent or highest priority) #2502

Closed timyates closed 1 week ago

timyates commented 2 weeks ago

Best reviewed commit by commit as I've tried to be strong and just fix one thing per commit...

Apologies for when I may have strayed 😢

This clears up a coupld of hundred Sonar errors and warnings


  1. JUnit5 class and method visibility

    In JUnit 5 it is recommendend to have package-private classes and methods for tests, Sonar flags this as an issue

  2. Logger should be a private static field

    Sonar flags this as a naming vilation (as it's all in caps), I think it just needed to be private final static

  3. Replace Collectors.toList() as appropriate

    Instead of stream.collect(Collectors.toList()), it is (since Java 16) recommended to use stream.toList(). Care must be taken though, as the returned list is then unmodifiable (which is different to the old Collectors version)

  4. assertEquals argument order

    AssertEquals in tests expects the first argument to be the expected value. Having it the wrong way round causes the output to be wrong, and can cause delays in debugging issues.

  5. accidental self assignement

    In 1 constructor, we were just setting the id to itself (as it wasn't passed as an argument) In the second (due to capitalization) we weren't using the passed value for Id. This wasn't an issue however as I don't believe the constructor is used 🤔

  6. CSS: deg instead of eg

    Sonar caught a typo

  7. CSS: duplicated padding member

    I don't believe the spec says what to do when it's declared twice (hence the warning in Sonar). I kept the second-most padding (which I believe is what most browsers do)

  8. CSS: invalid box-size parameter

    I believe this should be box-sizing and I've update it as such

  9. Remove unrequired Mockito eq() usage

    This is not required and makes the tests harder to read

  10. Remove parentheses round single lambda parameters