[디비 파기 | PMD] NullAssignment 외 3건

NullAssignment

우선순위 : 3

코드 중간에서 변수에 null을 할당하는 것은 좋지 않은 형태이다.

이런 형태의 할당은 종종 개발자들이 프로그램의 흐름을 이해하지 못하고 코딩을 하고 있다는 표시가 될 수 있다.

주의 : 이런 형태의 경우가 드물게는 garbage collectoin에 도움이 되는 경우도 있다. (물론 잘 이해하고 사용했을

경우) 이렇게 의도하고 사용한 경우에는 이 룰을 무시해도 좋다.

 

샘플코드

1 2 3 4 5 6 7

public void bar() {   Object x = null; // this is OK   x = new Object();      // big, complex piece of code here   x = null; // this is not required      // big, complex piece of code here }

 

부연 설명

이 경우 중요한 부부은 주석으로 표시된 ‘매우 크고 복잡한 코드’에 있다.

중간에 특정 변수를 null로 처리하는 경우 이를 제대로 인지하지 못하게 되면 뒤의 코드에서 null로 할당된

변수를 새로운 인스턴스로 할당하지 않고 바로 사용하게 될 소지가 있으며 이럴 경우 NullPointerException이

발생하게 될 것이다.

믈론 한 메소드 안에서 그렇게 길고 복잡한 코드를 사용하는 것 자체가 문제라면 문제지만…

OnlyOneReturn

우선순위 : 3

메소드에는 종료 포인트가 반드시 하나만 있어야 하며 이 종료 포인트는 반드시 메소드의 가장 마지막에

있어야 한다.

 

샘플코드 (나쁜 예)

1 2 3 4 5 6 7 8 9

public class OneReturnOnly1 {   public void foo(int x) {     if (x > 0) {       return "hey";   // first exit     }       return "hi"; // second exit   } }

 

부연 설명

이는 원칙적이고 일반적인 룰을 설명한 것이다.

흔히들 메소드 내에 if, if/else, while, for, try/catch등 각종 코드 블록이 만들어지고 각각의 블록에서

상황에 맞게 return문을 사용하는 경우가 많다. 상황에 따라 이렇게 할 수밖에 없는 경우도 있지만

이는 곧 코드의 복잡도(Cyclomatic Complexity)가 올라갔다는 반증이기도 하다.

가독성을 높이고 코드의 흐름을 원활하게 하기 위해서는 가능한한 return문은 하나만, 메소드의 가장

마지막에 위치할 수 있도록 하자.

AvoidLiteralsInIfCondition

우선순위 : 3

조건문의 조건식에 하드코딩 된 상수를 사용하지 말아라.

의미를 설명할 수 있는 이름을 가진 static 변수나 private 멤버 변수로 사용하는 것이 더 좋은 방법이다.

 

샘플코드

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20

private static final int MAX_NUMBER_OF_REQUESTS = 10;   public void checkRequests() {     if (i == 10) {                        // magic number, buried in a method       doSomething();     }       if (i == MAX_NUMBER_OF_REQUESTS) {    // 더 나은 접근방법       doSomething();     }       if (aString.indexOf('.') != -1) {}     // magic number -1, by default ignored       if (aString.indexOf('.') >= 0) { }     // alternative approach       if (aDouble > 0.0) {}                  // magic number 0.0       if (aDouble >= Double.MIN_VALUE) {}    // 더 나은 접근방법   }

 

부연설명

위의 샘플코드에 ‘magic number’라는 표현이 나오는데 이 것은 Refactoring에서도 자주 언급되는 내용으로

마치 마술처럼 어떻게도 해석이 될 수 있다는 것을 빗댄 표현이다.

즉, 상수를 하드코딩해 놓았을 때 그 숫자 자체만으로는 어떤 의미인지 이해가 불가능하다.

따라서 이러한 숫자들은 static이나 private 멤버변수로 만들어 의미가 있는 이름으로 사용하도록

권장하고 있다.

UseObjectForClearerAPI

우선순위 : 3

public 메소드를 만들 때는 API 사용 조건에 대해 생각해야만 한다.

메소드를 public으로 만들었다는 것은 다른 클래스들에서 이 메소드를 사용하게 하는 것이고 따라서

보다 포괄적이고 개선된 API를 제공하고자 하는 것이다.

 

이런 경우 너무 많은 정보를 단순 나열 형태의 String으로 전달하게 된다면 이 파라미터들을 하나의 Object로

묶어 이 Object를 인자로 전달하는 것이 좋다. 일단 코딩이 길어지고 복잡해지는 것이나 파라미터의 갯수가

많음으로 해서 어떤 인자가 어떤 위치에 있어야 하는지에 대한 혼동을 줄일 수 있을 뿐더러 만일 전달해야 하는

파라미터의 수나 종류가 변경될 경우 이 Object만 수정하면 되기 때문이다.

 

샘플코드

1 2 3 4 5 6 7 8 9 10 11 12 13 14

public class MyClass {   public void connect(String username,                                   String pssd,                                   String databaseName,                                   String databaseAdress)       // 아래와 같이 하나의 Object(class)로 만들어서 인자를 전달하면     // 보다 깔끔한 API를 만들 수 있고 또한 좀 더 투명하게(코드 변경 없이)     // 전달되는 데이터를 추가할 수 있다.     // void connect(UserData data);     {       } }