Tuesday, March 10, 2015

Static Import Check for Checkstyle

Static import: http://docs.oracle.com/javase/1.5.0/docs/guide/language/static-import.html

Command to catch code:
 grep --include=*.java --exclude={*Test.java,Test*.java,*IT.java} -rw -m 1 . -e "import static"

we can not forbid static import usage at all, it is useful, we need to limit it, So we will forbid all static imports that are not applying to following options:

1) allow in target member is used more then X times.
see example in point "3)" and imagine there is a lot of such code blocks, that option could allow static import for "psvBuilder".

2) allow in Class of target member could brake Line-limit more then X times.
 that it is not required as will hide too much code that should be formatted appropriately

3) Allow in complicated statements with a lot of inner methods calls in arguments.

import static com.commons.domain.DsvFileDefinition.psvBuilder;
import static com.commons.domain.FeedDefinition.feedBuilder;
import static com.commons.domain.ZipFileDefinition.zipBuilder;

private static FeedDefinition getFeedDefinition() {
return feedBuilder("Premium", "/datafeeds/", "NAME", "NAME1").addFile(
zipBuilder("premium").addFiles(of(
psvBuilder("address").build(),
psvBuilder("entity").build(),public enum BetPosition {
    // ...
    STRAIGHT_32(96, ...),
    STRAIGHT_33(142, ...),
    STRAIGHT_34(52, ...),
    BET_1_2(53, ...),
    BET_4_5(55, ...),
    // ... long list of enum values

psvBuilder("entity_changes").build(),
psvBuilder("entity_identifiers").build(),
psvBuilder("entity_names").build(),
psvBuilder("entity_profiles").build(),
psvBuilder("entity_relationships").build(),
psvBuilder("entity_structure").build(),
psvBuilder("entity_map").build(),
psvBuilder("listing_map").build())).build()
).build();


4) Ignore list of classes/methods that team presume distinctive and whole team know.
"com.google.common.base.Strings,com.google.common.base.XXXXXXX"

import static com.google.common.base.Strings.isNullOrEmpty;
import static com.google.common.base.Strings.nullToEmpty;

this.schemaName = nullToEmpty(schemaName);
if (isNullOrEmpty(tableName)) {
throw new IllegalArgumentException("tableName could not be empty");
}
this.tableName = tableName;

5)
Invalid usage:
import static com.google.common.collect.Iterables.getFirst;
....

TreeSet<Name> names = Sets.newTreeSet(new ILifeCycleEntityComparator());
if (names.isEmpty()) {
// do smth 
} else {
Name firstName = getFirst(tradenames, null);public enum BetPosition {
    // ...
    STRAIGHT_32(96, ...),
    STRAIGHT_33(142, ...),
    STRAIGHT_34(52, ...),
    BET_1_2(53, ...),
    BET_4_5(55, ...),
    // ... long list of enum values

// do smth else
}

6)
Here looks like static import allow to keep lines short.

import static com.revere.det.core.domain.validation.ValidationUtils.formatPeriod;

details.append("Focused sector: ").append(focusSector.getPath())
.append(", start date of focus: ").append(formatPeriod(focus.getStartDate()))
.append(", end date of focus: ").append(formatPeriod(focus.getEndDate()))
.append(NEW_LINE);

7)
Inappropriate - used only one time in whole file.

import static org.joda.time.LocalDateTime.now;

public class Builder extends DataBuilder {

private static final java.sql.Date THREE_YEARS_AGO = new java.sql.Date(now().minusYears(3).toDate().getTime());

=======================

https://code.google.com/p/guava-libraries/wiki/FunctionalExplained

Even using static imports, even if the Function and the Predicate declarations are moved to a different file, the first implementation is less concise, less readable, and less efficient.   

=================

Here's a shortened version of a class where static import is suitable:

===============================
import static com.some.company.BetPosition.*;

public class NumberMapping {

    private final BetPosition[][] numberToPositions = new BetPosition[][] {
            /* 0 number */{ STRAIGHT_0, BET_0_1, BET_0_2, BET_0_3, BET_0_1_2, BET_0_2_3, BET_0_1_2_3 },                 
            // ... 1-35 ...
            /* 36 number */{ STRAIGHT_36, BET_35_36, BET_33_36, BET_34_35_36, BET_32_33_35_36, BET_31_32_33_34_35_36,
                    TOP_2_TO_1, DOZEN_3_RD_12, HIGH_19_TO_36, RED, EVEN } };

    public BetPosition[] getPositions(int number) {
        BetPosition[] positions = numberToPositions[number];
        // some logic
        return positions;
    }
}


Where BetPosition is an enum with such a content:

public enum BetPosition {
    // ...
    STRAIGHT_32(96, ...),
    STRAIGHT_33(142, ...),
    STRAIGHT_34(52, ...),
    BET_1_2(53, ...),
    BET_4_5(55, ...),
    // ... long list of enum values

====================================

TODO..... investigate Guava code to have be sure that their code could follow that Check.

Issue is registered:
https://github.com/sevntu-checkstyle/sevntu.checkstyle/issues/453 

No comments:

Post a Comment