1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

【実装】コードの不吉な臭いを消滅

Last updated at Posted at 2020-07-02

1.性能

1-1.Mapのキーと値を取得する時、entrySet()を使う

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.Collection.isEmpty()を使う

bad.java
if (collection.size() == 0) {
    ...
}
good.java
if (collection.isEmpty()) {
    ...
}

1-3.集合自分を自分にセットしないでください。

bad.java
List<String> list = new ArrayList<>();
list.add("Hello");
list.add("World");
if (list.containsAll(list)) { // 意味がない、trueです。
    ...
}
list.removeAll(list); // 性能が悪く、clear()を使う。

1-4.集合初期化際にできるだけサイズを指定する。

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.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; // 問題なし、javaコンパイラは最適化できる
StringBuilder sb = new StringBuilder();
for (int i = 0; i < 10; i++) {
    sb.append(i);  // ループ中でjavaコンパイラは最適化できないので、手動StringBuilderを使う
}

1-6.Listのランダムな項目を返す

good.java
List<Integer> list = otherService.getList();
if (list instanceof RandomAccess) {
    // 内部的には配列で実現ですので、ランダムできる
    System.out.println(list.get(list.size() - 1));
} else {
    // 内部的にはリンクリストで実現するかもしれない、パフォーマンスが悪い
}

1-7.Collection.containsメソッドを頻繁に呼び出せれば、Setを使用する

bad.java
ArrayList<Integer> list = otherService.getList();
for (int i = 0; i <= Integer.MAX_VALUE; i++) {
    // 時間複雑度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++) {
    // 時間複雑度O(1)
    set.contains(i);
}

2.優雅なコード

2-1.long定数の後に大文字のLを追加

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

2-2.マジックナンバーを使わないでください

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.コレクションで静的メンバー変数を割り当てないでください

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おすすめ

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.未使用メンバやメソッドを削除してください

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.未使用ローカル変数を削除してください

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.未使用メソッドの引数を削除してください

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.余分な括弧を削除を削除してください

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.ユーティリティクラスのコンストラクタは遮断すべきです。

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.冗長な例外のキャッチとスローを削除してください

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.パブリック静的定数は、クラスを通じてアクセスすべきである。

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.「Null PointerException」でnullを判断しないでください。

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();
}

2-13.String.valueOfを使って、""+valueの代わりに使います。

bad.java
int i = 1;
String s = "" + i;
good.java
int i = 1;
String s = String.valueOf(i);

2-14.廃止コードに@Deprecatedコメントを追加してください。

good.java
/**
 * 保存
 *
 * @deprecated このメソッドは効率が悪いため、@link newSave()メソッドを使って置き換えてください。
 */
@Deprecated
public void save(){
    // do something
}

3.コードをバグから遠ざける

3-1.コンストラクターBigDecimal(double)の使用を禁止する

bad.java
BigDecimal value = new BigDecimal(0.1D); // 0.100000000000000005551115...
good.java
BigDecimal value = BigDecimal.valueOf(0.1D);; // 0.1

3-2.空の配列と空のコレクションを返します。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.定数または決定値を優先的に使用するequals方法を呼び出す。

bad.java
public void isFinished(OrderStatus status) {
    return status.equals(OrderStatus.FINISHED); // 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.列挙された属性フィールドはプライベートで不変でなければなりません

bad.java
public enum UserStatus {
    DISABLED(0, "無効"),
    ENABLED(1, "有効");

    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, "無効"),
    ENABLED(1, "有効");

    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.注意String.split(String regex)

bad.java
"a.ab.abc".split("."); // 結果は[]
"a|ab|abc".split("|"); // 結果は["a", "|", "a", "b", "|", "a", "b", "c"]
good.java
"a.ab.abc".split("\\."); // 結果は["a", "ab", "abc"]
"a|ab|abc".split("\\|"); // 結果は["a", "ab", "abc"]

後記

参考
アリババの開発者社区から翻訳

1
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?