[Implementation] Eliminate the ominous smell of code

1. Performance

1-1. Use entrySet () to get the key and value of Map

bad.java


Map<String, String> map = ...;
for (String key : map.keySet()) {
    String value = map.get(key);
    ...
}

good.java


Map<String, String> map = ...;
for (Map.Entry<String, String> entry : map.entrySet()) {
    String key = entry.getKey();
    String value = entry.getValue();
    ...
}

1-2. Use Collection.isEmpty ()

bad.java


if (collection.size() == 0) {
    ...
}

good.java


if (collection.isEmpty()) {
    ...
}

1-3. Set Do not set yourself.

bad.java


List<String> list = new ArrayList<>();
list.add("Hello");
list.add("World");
if (list.containsAll(list)) { //It doesn't make sense, it's true.
    ...
}
list.removeAll(list); //Poor performance, clear()use.

1-4. Specify the size as much as possible when initializing the set.

bad.java


int[] arr = new int[]{1, 2, 3};
List<Integer> list = new ArrayList<>();
for (int i : arr) {
    list.add(i);
}

good.java


int[] arr = new int[]{1, 2, 3};
List<Integer> list = new ArrayList<>(arr.length);
for (int i : arr) {
    list.add(i);
}

1-5. Concatenate strings with StringBuilder.

bad.java


String s = "";
for (int i = 0; i < 10; i++) {
    s += i;
}

good.java


String a = "a";
String b = "b";
String c = "c";
String s = a + b + c; //No problem, java compiler can be optimized
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 10; i++) {
    sb.append(i);  //Use manual StringBuilder as the java compiler cannot be optimized inside the loop
}

1-6. Returns a random item in List

good.java


List<Integer> list = otherService.getList();
if (list instanceof RandomAccess) {
    //Internally it is realized by an array, so it can be randomized
    System.out.println(list.get(list.size() - 1));
} else {
    //Poor performance that may be achieved internally with linked lists
}

1-7. Use Set if you can call the Collection.contains method frequently

bad.java


ArrayList<Integer> list = otherService.getList();
for (int i = 0; i <= Integer.MAX_VALUE; i++) {
    //Time complexity O(n)
    list.contains(i);
}

good.java


ArrayList<Integer> list = otherService.getList();
Set<Integer> set = new HashSet(list);
for (int i = 0; i <= Integer.MAX_VALUE; i++) {
    //Time complexity O(1)
    set.contains(i);
}

2. Elegant chord

2-1. Add uppercase L after long constant

bad.java


long value = 1l;
long max = Math.max(1L, 5);

good.java


long value = 1L;
long max = Math.max(1L, 5L);

2-2. Do not use magic numbers

bad.java


for (int i = 0; i < 100; i++){
    ...
}
if (a == 100) {
    ...
}

good.java


private static final int MAX_COUNT = 100;
for (int i = 0; i < MAX_COUNT; i++){
    ...
}
if (count == MAX_COUNT) {
    ...
}

2-3. Do not assign static member variables in the collection

bad.java


private static Map<String, Integer> map = new HashMap<String, Integer>() {
    {
        put("a", 1);
        put("b", 2);
    }
};

private static List<String> list = new ArrayList<String>() {
    {
        add("a");
        add("b");
    }
};

good.java


private static Map<String, Integer> map = new HashMap<>();
static {
    map.put("a", 1);
    map.put("b", 2);
};

private static List<String> list = new ArrayList<>();
static {
    list.add("a");
    list.add("b");
};

2-4.try-with-resources recommended

bad.java


private void handle(String fileName) {
    BufferedReader reader = null;
    try {
        String line;
        reader = new BufferedReader(new FileReader(fileName));
        while ((line = reader.readLine()) != null) {
            ...
        }
    } catch (Exception e) {
        ...
    } finally {
        if (reader != null) {
            try {
                reader.close();
            } catch (IOException e) {
                ...
            }
        }
    }
}

good.java


private void handle(String fileName) {
    try (BufferedReader reader = new BufferedReader(new FileReader(fileName))) {
        String line;
        while ((line = reader.readLine()) != null) {
            ...
        }
    } catch (Exception e) {
        ...
    }
}

2-5. Delete unused members and methods

bad.java


public class DoubleDemo1 {
    private int unusedField = 100;
    private void unusedMethod() {
        ...
    }
    public int sum(int a, int b) {
        return a + b;
    }
}

good.java


public class DoubleDemo1 {
    public int sum(int a, int b) {
        return a + b;
    }
}

2-6. Delete unused local variables

bad.java


public int sum(int a, int b) {
    int c = 100;
    return a + b;
}

good.java


public int sum(int a, int b) {
    return a + b;
}

2-7. Delete the unused method argument

bad.java


public int sum(int a, int b, int c) {
    return a + b;
}

good.java


public int sum(int a, int b) {
    return a + b;
}

2-8. Remove extra parentheses Remove

bad.java


return (x);
return (x + 2);
int x = (y * 3) + 1;
int m = (n * 4 + 2);

good.java


return x;
return x + 2;
int x = y * 3 + 1;
int m = n * 4 + 2;

2-9. Utility class constructors should be blocked.

bad.java


public class MathUtils {
    public static final double PI = 3.1415926D;
    public static int sum(int a, int b) {
        return a + b;
    }
}

good.java


public class MathUtils {
    public static final double PI = 3.1415926D;
    private MathUtils() {}
    public static int sum(int a, int b) {
        return a + b;
    }
}

2-10. Remove redundant exception catches and throws

bad.java


private static String readFile(String fileName) throws IOException {
    try (BufferedReader reader = new BufferedReader(new FileReader(fileName))) {
        String line;
        StringBuilder builder = new StringBuilder();
        while ((line = reader.readLine()) != null) {
            builder.append(line);
        }
        return builder.toString();
    } catch (Exception e) {
        throw e;
    }
}

good.java


private static String readFile(String fileName) throws IOException {
    try (BufferedReader reader = new BufferedReader(new FileReader(fileName))) {
        String line;
        StringBuilder builder = new StringBuilder();
        while ((line = reader.readLine()) != null) {
            builder.append(line);
        }
        return builder.toString();
    }
}

2-11. Public static constants should be accessed through the class.

bad.java


public class User {
    public static final String CONST_NAME = "name";
    ...
}

User user = new User();
String nameKey = user.CONST_NAME;

good.java


public class User {
    public static final String CONST_NAME = "name";
    ...
}

String nameKey = User.CONST_NAME;

2-12. Do not judge null by "Null Pointer Exception".

bad.java


public String getUserName(User user) {
    try {
        return user.getName();
    } catch (NullPointerException e) {
        return null;
    }
}

good.java


public String getUserName(User user) {
    if (Objects.isNull(user)) {
        return null;
    }
    return user.getName();
}

Use 2-13.String.valueOf instead of "" + value.

bad.java


int i = 1;
String s = "" + i;

good.java


int i = 1;
String s = String.valueOf(i);

2-14. Add the @Deprecated comment to the deprecated code.

good.java


/**
 *Save
 *
 * @deprecated Because this method is inefficient@link newSave()Replace using the method.
 */
@Deprecated
public void save(){
    // do something
}

3. Keep code away from bugs

3-1. Prohibit the use of the constructor BigDecimal (double)

bad.java


BigDecimal value = new BigDecimal(0.1D); // 0.100000000000000005551115...

good.java


BigDecimal value = BigDecimal.valueOf(0.1D);; // 0.1

3-2. Returns an empty array and an empty collection. Not null.

bad.java


public static Result[] getResults() {
    return null;
}

public static List<Result> getResultList() {
    return null;
}

public static Map<String, Result> getResultMap() {
    return null;
}

public static void main(String[] args) {
    Result[] results = getResults();
    if (results != null) {
        for (Result result : results) {
            ...
        }
    }

    List<Result> resultList = getResultList();
    if (resultList != null) {
        for (Result result : resultList) {
            ...
        }
    }

    Map<String, Result> resultMap = getResultMap();
    if (resultMap != null) {
        for (Map.Entry<String, Result> resultEntry : resultMap) {
            ...
        }
    }
}

good.java


public static Result[] getResults() {
    return new Result[0];
}

public static List<Result> getResultList() {
    return Collections.emptyList();
}

public static Map<String, Result> getResultMap() {
    return Collections.emptyMap();
}

public static void main(String[] args) {
    Result[] results = getResults();
    for (Result result : results) {
        ...
    }

    List<Result> resultList = getResultList();
    for (Result result : resultList) {
        ...
    }

    Map<String, Result> resultMap = getResultMap();
    for (Map.Entry<String, Result> resultEntry : resultMap) {
        ...
    }
}

3-3. Call the equals method that preferentially uses constants or determined values.

bad.java


public void isFinished(OrderStatus status) {
    return status.equals(OrderStatus.FINISHED); //May be Null PointerException
}

good.java


public void isFinished(OrderStatus status) {
    return OrderStatus.FINISHED.equals(status);
}

public void isFinished(OrderStatus status) {
    return Objects.equals(status, OrderStatus.FINISHED);
}

3-4. The listed attribute fields must be private and immutable

bad.java


public enum UserStatus {
    DISABLED(0, "Invalid"),
    ENABLED(1, "Effectiveness");

    public int value;
    private String description;

    private UserStatus(int value, String description) {
        this.value = value;
        this.description = description;
    }

    public String getDescription() {
        return description;
    }

    public void setDescription(String description) {
        this.description = description;
    }
}

good.java


public enum UserStatus {
    DISABLED(0, "Invalid"),
    ENABLED(1, "Effectiveness");

    private final int value;
    private final String description;

    private UserStatus(int value, String description) {
        this.value = value;
        this.description = description;
    }

    public int getValue() {
        return value;
    }

    public String getDescription() {
        return description;
    }
}

3-5. Attention String.split (String regex)

bad.java


"a.ab.abc".split("."); //Result is[]
"a|ab|abc".split("|"); //Result is["a", "|", "a", "b", "|", "a", "b", "c"]

good.java


"a.ab.abc".split("\\."); //Result is["a", "ab", "abc"]
"a|ab|abc".split("\\|"); //Result is["a", "ab", "abc"]

Postscript

Reference Translated from Alibaba Developer District

Recommended Posts

[Implementation] Eliminate the ominous smell of code
Implementation of unit test code
Ominous odor list suggesting the possibility of refactoring
RSpec-Results of reviewing the test code for'users validation'
[Ruby] Code to display the day of the week
Implementation of GKAccessPoint
A review of the code used by rails beginners
Implement the UICollectionView of iOS14 with the minimum required code.
The world of clara-rules (2)
Implementation of flash messages
Implementation of search function
Judgment of the calendar
The world of clara-rules (1)
The world of clara-rules (3)
Applied implementation of chat-space
The idea of quicksort
Implementation of pagination function
The idea of jQuery
[Java] Simplify the implementation of data history management with Reladomo
[Ruby] Creating code using the concept of classes and instances
Traps brought about by the default implementation of the Java 8 interface
Specify the character code of the source when building with Maven