이번 블랙잭 미션을 진행하면서 Sonarqube를 통해 발견한 코드 악취에 대해 간단하게 공유해보고자 합니다.

추상 클래스의 public 생성자를 제거하라

추상 클래스의 생성자는 하위 클래스들에 의해서만 호출됩니다.
따라서 public 생성자가 있을 필요가 없습니다.

물론 public이어도 문제는 없습니다.
그래도 외부에서 호출을 시도하는 행위를 원천적으로 차단하기 위해 최대 protected까지만 사용하는 것을 권장합니다.

추상 클래스에 정의한 공통 메소드에 final을 고려하라

추상 클래스에서 정의한 공통 메소드 또한 하위 클래스에서 재정의가 가능합니다.

이러한 공통 메소드들이 이 추상 클래스에서는 공통적으로 사용되며, 더 이상 메소드의 기능을 특화하고 싶지 않다면 final 키워드를 추가하는 것도 고려할 수 있습니다.

final 키워드를 추가해 오버라이딩이 불가능함을 명시하면 의도를 조금 더 명확하게 드러낼 수 있습니다.

유틸 클래스에 public 생성자를 제거하라

유틸 클래스는 단순히 기능을 제공하기 위한 static 메소드만이 추가된 클래스입니다.

즉 유틸성 목적을 가진 클래스라고 볼 수 있습니다.

특정 클래스를 유틸 클래스로 사용할지 빈으로 사용할지 고려하는 것은 고려 사항이 많은 부분입니다.
다만 일단 유틸 클래스로 사용하겠다고 결정했다면 public 생성자를 제거하는 것을 권장합니다.

public final SomethingUtil {

    private SomethingUtil() {
    }
    
    public static void something() {
        ...
    }
}

또한 클래스 레벨에 final을 붙여 해당 클래스를 확장도, 생성도 못하게 명시적으로 표현한다면 조금 더 명확하게 클래스의 용도를 표현할 수 있습니다.

테스트에서 public 접근 제어자를 제거하라

JUnit4에서는 테스트 실행을 위해 클래스와 메소드의 접근 제어자가 public이어야 했습니다.

JUnit5에서는 테스트 실행을 위해 리플렉션을 사용하기 때문에 더 이상 public 접근 제어자는 필요가 없어졌습니다.

그러므로 public 접근 제어자를 제거해 외부에서 호출할 수 있는 일말의 가능성을 지우고, 코드의 양을 줄이는 것을 권장합니다.

Assertions에서 제공하는 메소드를 적극 활용해라

Assertions에서 제공해주는 특화된 메소드를 사용한다면 가독성과 편의성을 더 올릴 수 있습니다.

자주 사용될만한 메소드들은 다음과 같습니다.

이전	이후
assertThat(getObject()).isEqualTo(null)	assertThat(getObject()).isNull()
assertThat(getBoolean()).isEqualTo(true)	assertThat(getBoolean()).isTrue()
assertThat(getBoolean()).isEqualTo(false)	assertThat(getBoolean()).isFalse()
assertThat(x.equals(y)).isTrue()	assertThat(x).isEqualTo(y)
assertThat(x == y).isTrue()	assertThat(x).isSameAs(y)
assertThat(x == null).isTrue()	assertThat(x).isNull()
assertThat(x.toString()).isEqualTo(y)	assertThat(x).hasToString(y)
assertThat(x.compareTo(y)).isZero()	assertThat(x).isEqualByComparingTo(y)
assertThat(x >= y).isGreaterThanOrEqualTo(0)	assertThat(x).isGreaterThanOrEqualTo(y)
assertThat(x > y).isPositive()	assertThat(x).isGreaterThan(y)
assertThat(x <= y).isNotPositive()	assertThat(x).isLessThanOrEqualTo(y)
assertThat(x < y).isTrue()	assertThat(x).isLessThan(y)
assertThat(x).isGreaterThanOrEqualTo(0)	assertThat(x).isNotNegative()
assertThat(getString().isEmpty()).isTrue()	assertThat(getString()).isEmpty()
assertThat(getString()).hasSize(0)	assertThat(getString()).isEmpty()
assertThat(getString().equals(expected)).isTrue()	assertThat(getString()).isEqualTo(expected)
assertThat(getString().equalsIgnoreCase(expected)).isTrue()	assertThat(getString()).isEqualToIgnoringCase(expected)
assertThat(getString().contains(expected)).isTrue()	assertThat(getString()).contains(expected)
assertThat(getString().startsWith(expected)).isTrue()	assertThat(getString()).startsWith(expected)
assertThat(getString().endsWith(expected)).isTrue()	assertThat(getString()).endsWith(expected)
assertThat(getString().matches(expected)).isTrue()	assertThat(getString()).matches(expected)
assertThat(getString().trim()).isEmpty()	assertThat(getString()).isBlank()
assertThat(getString().length()).isEqualTo(length)	assertThat(getString()).hasSize(length)
assertThat(getString().length()).hasSize(expected.length())	assertThat(getString()).hasSameSizeAs(expected)

더 많은 예시를 확인하고 싶으시다면 링크를 확인해주세요.

기본 패키지는 사용하지 마라

이름이 없는 기본 패키지(root 디렉토리에 존재하는 패키지)는 소규모 혹은 임시로 애플리케이션을 개발하거나 개발을 막 시작할 때 편의를 위한 용도입니다.

이러한 내용은 Java에서 적용한 사례이기 때문에 Java 1.4이후에는 기본 패키지에 존재하는 클래스는 다른 클래스에서 접근할 수 없습니다.

인스턴스 필드의 이름은 클래스의 이름을 포함하지 마라

클래스와 동일한 이름(대소문자 차이 제외)을 가진 인스턴스 필드가 있는 것은 다른 개발자에게 혼란을 일으킬 수 있습니다.

이는 클래스 자체에 대한 인스턴스 필드의 이름을 지정하는 일반적인 관행을 고려할 때 특히 그렇습니다.

조금 더 의미를 드러내거나, 원시 값을 포장한 객체라면 value로 표현하는 것도 고려해볼 수 있습니다.

랜덤한 정수 값을 생성할 때 Math.random() 메소드를 사용하지 마라

기본적으로 부동 소수점을 반환하는 Math.random()을 통해 랜덤한 정수 값을 생성하고자 한다면 복잡한 로직이 필요합니다.

이로 인해 치명적인 버그가 발생할 수 있습니다.

그러므로 Random.nextInt()와 같이 정수 타입을 무작위로 반환하는 메소드를 사용할 것을 권장합니다.

메소드 지역 변수와 클래스의 인스턴스 필드 이름을 동일하게 짓지 마라

외부 범위(인스턴스 필드)에서 선언된 변수를 재정의하거나 가리면(Shadow) 해당 코드 블록의 가독성과 유지 관리 가능성에 큰 영향을 미칠 수 있습니다.

또한 개발자가 하나의 변수를 사용하고 있다고 생각하지만 실제로는 다른 변수를 사용하고 있기 때문에 버그가 발생할 수 있습니다.