[未来バックエンド開発者日記]700個のコードの悪臭を処理して学んだこと

最近、会社はプロジェクトにソフトウェアやキューブを設置し、中間の残りの時間にコード消臭作業を行っています.今日はこれらの悪臭を取り除く過程で学んだことを整理します.

1.抽象クラスの公開ジェネレータを削除する

Abstract classes should not have public constructors. Constructors of abstract classes can only be called in constructors of their subclasses. So there is no point in making them public. The protected modifier should be enough.
抽象クラスのジェネレータは、サブクラスからのみ呼び出されます.そのため、作成者を公開する必要はありません.publicもよくコンパイルして実行できますが、最大保護のみが使用されます.

2.ユーティリティクラスの公開ジェネレータを削除する

Utility classes should not have public constructors
ユーティリティクラスは、静的メソッドのみからなる実用的な目的を持つクラスです.特定のオブジェクトをユーティリティクラスとして使用するか、空として使用するかを選択するのはもう難しい問題ですが、ユーティリティクラスを使用することを決定した場合は注意が必要です.公開ジェネレータを削除します.

// Bad!!
// naive한 유틸 클래스. 상속과 인스턴스화가 가능하다 
public class UtilClass {

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

}

// Good!!
// 상속을 막고 인스턴스화를 막은 유틸 클래스 
public final class UtilClass {

    private UtilClass() {
        throw new IllegalStateException("must not intantiate util class");
    }

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

}

// Good!!
// lombok을 사용한다면 간단하게 애노테이션 하나를 붙여 위와 동일한 코드를 생성할 수 있다 
@UtilityClass
public class UtilClass {

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

}

「そこまでやろうか」そう考えることはできますが、ユーティリティクラスの特性を考慮して多くの場所で利用できます.作成者はもちろん、変なものは使わないことはよく知っていますが、上記のクラスを使うすべての人がそうする保証はありません.

3.テストからパブリックコントローラを削除する

JUnit5 is more tolerant regarding the visibilities of Test classes than JUnit4, which required everything to be public.
In this context, JUnit5 test classes can have any visibility but private, however, it is recommended to use the default package visibility, which improves readability of code.
JUnit 4はクラスとメソッドを公開してテストを実行する必要があります.でもJUnit 5に来た時は反応が活発だったのでそれ以上する必要はありませんでしたこれが他のテストで使用されていない場合は、共通のコントロールを削除し、パッケージ専用のコントロールを使用します.コントロールを追加する必要がないため、コードの読み取りが向上します.

// Bad!!
public class Test {

    @BeforeEach
    public void setup() {
        ...
    }

    @Test
    public void test() {
        ...
    }

}

// Good!!
class Test {

    @BeforeEach
    void setup() {
        ...
    }

    @Test
    void test() {
        ...
    }

}

4.複数のサポートAPIを利用する

より特殊な場合の方法(例えば、assertThat().containsEntry()またはassertThat().isZero())によって毒性が向上する.

// Bad!!
assertThat(sum).isEqualTo(0L)

assertThat(formDataRequest.get("file")).isEqualTo(mockUrl);
assertThat(formDataRequest.get("command")).isEqualTo(CommandType.SOME_COMMAND.name());
            
// Good!!
assertThat(upload).isZero();

assertThat(bizhowsDesignResourceSaveRequest)
        .containsEntry("file", mockUrl)
        .containsEntry("command", CommandType.SOME_COMMAND.name());

5.上司は常に黙っている

Making a constant just final as opposed to static final leads to duplicating its value for every instance of the class, uselessly increasing the amount of memory required to execute the application.
定数をstaticとして宣言しない場合、各オブジェクトインスタンスには対応するフィールドがあります.これにより不要なメモリが使用されるため、定数であれば常にfinalが加算されます.

// Bad!!
public class Myclass {    
    public final int THRESHOLD = 3;  
}  

// Good!!
public class Myclass {    
    public static final int THRESHOLD = 3; 
}  

6.オプションを単純nullチェックに使用しない

Optionalは、方法のない結果を示すために作られたものです.単純nullチェックに使いすぎないか考えてみましょうOptionalを正しく使いたいなら?->Java Optionを正しく使用

// Bad!!
public void someMethod(SomeObject object) {
    Optional.ofNullable(object).orElseThrow(() -> new NullPointerException());
    ...
}

// Good!!
public void someMethod(SomeObject object) {
    if (object == null) {
        throw new NullPointerException();
    }
    ...
}

上記のコードは、オプションの意図とは異なり、nullでなければオブジェクトを返すために使用されません.上記の場合null検査を明示的に行うだけです

7.少なくとも1つのassert文を作成する

テストコードにassert文を書くのは当然だと思うかもしれませんが、そうしない場合もあります.検証可能なサイドフレームがなく、異常を投げ出さないようにテストしたい場合は、どうすればいいですか?Junit 5のassertDoesNotThrow()を利用します.名前だけで何をする方法なのかはっきりしています.assert文がないよりも,テストを読む人に意図を明確に伝えることができる.

class Sut {
    void method(int param) {
        if (param == 0) {
            throw new RuntimeException();
        }
    }
}

// Bad!!
@Test
void test() {
    int param = 1;
    
    sut.method(param);
}

// Good!!
@Test
void test() {
    int param = 1;
    
    Executable execSut = () -> sut.method(int);
    
    assertDoesNotThrow(execSut);
}

8.assertThrowsでメソッドを1つだけ呼び出す

assertThrows()に送信されたコードが複数の場所で異常が発生する可能性がある場合、偽陽性が発生しやすく、何をテストするか分かりにくい.

// Before
    @Test
    void test() {
        assertThrows(SomeException.class, () -> {
            SomeObject some = new SomeObjcet();
            some.setIdx(0);
            some.checkValidRequest(1234);  // 실제 테스트하려는 코드
        });
    }

// After
private final SomeObject sut = new SomeObjcet();

    @Test
    void test() {
        sut.setIdx(0);
    
        assertThrows(SomeException.class, 
                () -> sut.checkValidRequest(1234));
    }

9.テスト対象システムの名前をsutとする

これはスメルじゃない個人的にはいいと思うフィールドの名前を付けるときは、テストターゲットシステムの名前をsutとし、首に名前を書きましょう.コードがより簡潔になります.mockで始まるオブジェクト変数名が好きなら、そうすることもできます.sutに切り替えるだけで、実行コードとなるコードを簡単に読むことができます.特にコード長をテストする際には、いくつかの役に立つ経験があります.

// 이름이 길고 테스트 대상 시스템을 구분하기 어렵다 
class Test {

    @InjectMocks
    SomeObject someObjectTryingToTest;
    
    @Mock
    DependObject1 mockDependObject1;
    
    @Mock
    DependObject2 mockDependObject2;
    
    @Test
    void test() {
        given(mockDependObject1.getOne()).willReturn(Object);
        given(mockDependObject2.getList()).willReturn(List.of(Object));
        
        someObjectTryingToTest.doWork();
        
        assertThat(...);
        assertThat(...);
    }

}

// Good!!
class Test {

    @InjectMocks
    SomeObject sut;
    
    @Mock
    DependObject1 dependObject1;
    
    @Mock
    DependObject2 dependObject2;
    
    @Test
    void test() {
        given(dependObject1.getOne()).willReturn(Object);
        given(dependObject2.getList()).willReturn(List.of(Object));
        
        sut.doWork();
        
        assertThat(...);
        assertThat(...);
    }

}