Feuermagier / autograder

Automatic grading of student's Java code
MIT License
13 stars 7 forks source link

When should one replace sequential `.add` with an `.addAll`? #505

Open Luro02 opened 2 months ago

Luro02 commented 2 months ago

Description

The current implementation detects something like this:

list.add("a");
list.add("b");
list.add("c");
list.add("d");
list.add("e");

and suggests replacing that with

list.add(List.of("a", "b", "c", "d", "e"));

Depending on how large the content of the .add calls is, one will end up with a multi-line .addAll call that is not any better than the original one.

What to lint instead?

Most importantly this should not subtract points. Therefore, it must be separated from the other .addAll lint;

for (var value : collection) {
    list.add(value);
}
// -> replace with list.addAll(collection);

If the .add has a constant value, one could suggest replacing this with

private static final List<String> MY_CONSTANTS = List.of("a", "b", "c", "d", "e");

// ...

list.add(MY_CONSTANTS);

Note: Do not suggest this for classes or things that reference the current method. Enum Literals are okay.