[한글화 시리즈-07] Design Rules(1)

Design Rules(1)

이 룰셋은 클래스 디자인 시 고려할만한 룰을 포함한다.

UseSingleton

Static 메소드들로만 이루어진 클래스와 같은 경우에 Singleton 패턴을 이용하여 구현해야 한다. 만약 Singleton 패턴을 이용하여 구현할 시에는 모든 메소드는 non-static 이어야 하며, 생성자는 private으로 접근제한하여 외부에서 객체 생성(instantiation)을 하는 것을 막아야한다. 하지만 abstract class에는 적용할 수 없다.

//나쁜 예
//모든 메소드가 static 메소드이다.
public class MaybeASingleton {
  public static void foo() {}
  public static void bar() {}
}

//좋은 예
//내부 클래스를 이용하여 singleton 구현
public class Foo {
    private Foo(){
    }
    private static class FooHolder{
        static final Foo single = new Foo();
    }
    public static Foo getInstatnce(){
        return Foo.single;
    }
}

//스레드 동기화를 이용한 방법 나쁘지 않음
public class Foo {
    private volatile static Foo single;
    public static Foo getInstance(){
        //스레드 동기화
        synchronized(Foo.class) {
            if (single == null) {
                single = new Foo();
            }
        }
        return single;
    }
    private Foo(){
    }
}

//Spring framework DI를 이용하는 방법
@Component("foo")
public class Foo {
    private Foo(){}
}

SimplifyBooleanReturns

단지 boolean을 반환할 경우 불필요한 if ... then ... else를 사용하지 말자.

public class Foo {
  private int bar =2;
  public boolean isBarEqualsTo(int x) {
    // 좋지 않은 코드
    if (bar == x) {
     return true;
    } else {
     return false;
    }
    // 다음과 같이 변경될 수 있다.
    // return bar == x;
  }
}

SimplifyBooleanExpressions

불필요한 boolean 비교를 피하라. 간단한 코드를 복잡하게 만든다.

public class Bar {
  // 그냥 간단하게 다음과 같이 쓰자
  // bar = isFoo();
  private boolean bar = (isFoo() == true);

  public isFoo() { return false;}
}

SwitchStmtsShouldHaveDefault

Switch는 default 레이블을 꼭 포함해야한다.

public class Foo {
  public void bar() {
    int x = 2;
    switch (x) {
    //x가 2인 상황만 처리하여, 버그를 유발할 수 있다. 
    case 2: int j = 8;
    }

    switch (x) {
    //x가 2가 아니라도 default로 처리 가능
    case 2: 
      int j = 8;
      break;
    default:
      int j = 0;
      break;
    }
  }
}

AvoidDeeplyNestedIfStmts

너무 복잡한 중첩 조건문은 사용하지 말자. 읽기 힘들다.

public class Foo {
 public void bar(int x, int y, int z) {
  if (x>y) {
   if (y>z) {
    if (z==x) {
     // 너무 복잡해!!
    }
   }
  }
 }
}

AvoidReassigningParameters

전달 인자로 전달된 변수는 임시 지역 변수로 전달 받아 사용해야 한다.

public class Foo {
 private void foo(String bar) {
  bar = "something else";
 }
}

SwitchDensity

스위치의 각 case 내의 코드가 너무 많은 작업을 하는 것을 피해야 한다. 각 case내에 코드가 길어진다면 (기본 10줄), 새로운 메소드를 생성하거나 서브클래스(subclass)를 생성하여 사용해야한다.

public class Foo {
 public void bar(int x) {
   switch (x) {
     case 1: {
       // 너무 많은 코드 라인 - 새로운 메소드 또는 서브클래스를 사용하자
       break;
     } case 2: {
       // 너무 많은 코드 라인 - 새로운 메소드 또는 서브클래스를 사용하자
       break;
     }
   }
 }
}

ConstructorCallsOverridableMethod

생성자가 오버라이드 가능한 (overridable) toString 같은 메소드들을 호출한다면, 불완전하게 생성된 오브잭트를 호출하는 위험이 있을 수 있으며, 이를 알아차리기 힘들다. 이런 경우, 서브클래스가 자신의 슈퍼클래스를 생성할 수 없게 하거나, 서브클래스 내에 모든 생성 프로세스를 복사해야만 하므로, super()를 호출할 수 없게된다. 다음의 예로 자세한 내용을 설명한다.

public class SeniorClass {
  public SeniorClass(){
    //toString을 자식 클래스에서 오버라이딩 했을 경우 
    //NullPointerException이 발생할 가능성이 있다.
    toString();
  }
  public String toString(){
    return "IAmSeniorClass";
  }
}
public class JuniorClass extends SeniorClass {
  private String name;
  public JuniorClass(){
    //부모 클래스의 생성자을 호출할 때 
    //toString은 이미 자식 클래스의 toString으로 대체된 상태이다.
    //그러므로 name은 아직 null인 상태이고
    //.toUpperCase() 실행 시 NullPointerException를 발생시킨다.
    super(); 
    name = "JuniorClass";
  }
  public String toString(){
    return name.toUpperCase();
  }
}

AccessorClassGeneration

내부 클래스의 private으로 접근제한된 생성자는 접근자 메소드 생성을 필요하다. 이런식으로 외부 클래스를 접근하여 내부 클래스를 생성하는 방식은 사실상 인터페이스와 같은 역활을 하는 것이임으로, 이는 factory method 또는 private으로 접근 제한되지 않은 생성자로 해결할 수 있다. 또한 이런 방식은 package 범위내에 파악이 안되는 숨겨진 상태로 남을 것이며 이를 파악하는 것은 매우 어렵다.

public class Outer {
  void method(){
    //이런식으로 클래스를 생성하는 방식은 package 구조에서 파악하기
    //매우 어렵고, factory method를 이용하여 구현하는 것이 더 나은 방법이다.
    Inner ic = new Inner();
  }
  public class Inner {
    private Inner(){}
  }
}

FinalFieldCouldBeStatic

final로 설정된 필드(field)는 컴파일 시에 자동으로 static으로 전환된다. 그러므로 각 객체의 실행 시간을 줄이기 위하여 final 필드는 static도 같이 설정하는 것이 좋다.

public class Foo {
 public final int BAR = 42; // static final이 더욱 효율적이다.
}

CloseResource

모든 Connection, Statement 그리고 ResultSet 객체들과 같은 리소스들을 사용한 후에는 무조건 close를 해야한다. 하지만 JDK 7의 파일 기능등에서 close를 하지 않도록 수정된 부분들은 꼭 확인하자.

//JDK 6 이하
public class Bar {
  public void foo() {
    Connection c = pool.getConnection();
    try {
      ...
    } catch (SQLException ex) {
      // 에러처리는 여기에
    } finally {
      //리소스를 사용하면 꼭 닫자!
      c.close();
    }
  }
}

//하지만 다음과 같이 JDK7에서는 몇몇 리소스는 close를 쓰지 않는다.
public class Bar {
  public void foo() {
    try (BufferedReader br = new BufferedReader(new FileReader("C:\\testing.txt"))) {
      String line;
      while ((line = br.readLine()) != null) {
        ...
        
      }
    } catch (IOException e) {
    //에러처리
    } 
  }
}

NonStaticInitializer

nonstatic 초기화 블록(initializer block) 생성자가 호출되기 이전에 호출된다. 그러므로 이 블록은 드물게 사용되며, 인스턴스 생성지 혼란스러울 수 있다.

public class MyClass {
  // 이 초기화 블록은 생성자 이전에 호출된다.
  // 이 블록은 생성자가 아니므로 혼동하지 말자!
  {
    System.out.println("난 나스스로 생성할 수 없어!");
  }
}

DefaultLabelNotLastInSwitchStmt

관례상 switch의 default 레이블은 switch의 가장 마지막에 위치해야한다.

public class Foo {
  void bar(int a) {
    switch (a) {
      case 1: 
        ...
        break;
      default:  // 관례상 default는 가장 마지막에
        break;
      case 2:
        break;
    }
  }
}

NonCaseLabelInSwitchStatement

switch안에 case가 아닌 다른 레이블(named break/continue)이 있어도 오류는 아니다. 하지만 혼란스럽다.

public class Foo {
  void bar(int a) {
    switch (a) {
      case 1:
        ...
        break;
      mylabel: // 쓸 수는 있다. 하지만 혼란스럽다!
        break;
      default:
        break;
    }
  }
}

OptimizableToArrayCall

Collection.toArray를 호출할 경우 원하는 타입의 배열을 인자로 전달해야 하는데, 이 경우 비어있는 배열보다는 길이를 설정한 배열이 더욱 효율적이다.

class Foo {
  void bar(Collection x) {
    // 덜 효율적이다.
    x.toArray(new Foo[0]);
    // 배열의 사이즈를 설정하는 것이 더욱 효과적이며,
    // 몇 몇 collection implementation들의 반복되는 호출을 막을 수 있다.
    x.toArray(new Foo[x.size()]);
  }
}

BadComparison

변수를 Double.NaN과 비교하는 것은 로직 오류를 발생시키는 경향이 있으므로 피해야 한다.

public class Bar {
  //Double.NaN비교는 로직 오류를 발생시킨다.
  boolean x = (y == Double.NaN);
}

EqualsNull

경험이 부족한 프로그래머들은 비교에 대한 개념이 부족해서 equals()를 이용해서 null을 비교하는 경우가 있다. ==은 객체 참조(object reference)를 직접 비교하지만. equals()는 객체 내에 정의된 방식에 따라 비교한다. 만약 equals()를 실행하는 객체가 null일 경우 NullPointerException을 발생시킨다.

class Bar {
  void foo() {
    String x = "foo";
    if (x.equals(null)) { // 나쁜 방식이다. x == null로 충분하다.
      doSomething();
    }
  }
}

ConfusingTernary

if else 절에 부정연산자(!)로 비교하는 것은 피해야 한다. 예를들면, if(x != y) { diff(); } else { same(); } 은 if(x == y) { same(); } else { diff(); }과 같고, 대부분의 if(x != y)는 else가 없이 return을 실행하는 경우가 종종 있다. 가독성 있는 사용을 위해서는 부정연산자를 먼저 사용하지 않는 것이 좋으며, 이는 또한 오류를 먼저 처리할지 정상적인 코드를 먼저 처리할지에 대한 사소한 순서 문제도 해결할 수 있다.

public class Foo {
  boolean ternary(int x, int y) {
    //다음은 return (x == y) ? same : diff; 로 변환 하는 것이 좋다
    return (x != y) ? diff : same;
  }

  void ifElse(int x, int y) {
    //이런 경우 x == y를 먼저 처리하는 것이 좋다.
    if(x != y) {
      diff();
    } else {
      same();
    }
  }
}

InstantiationToGetClass

단지 getClass()를 호출하기 위해서 객체를 인스턴트화(instantiating) 할 필요 없다. 단지 클래스의 public 맴버인 .class를 사용하자.

public class Foo {
  // 이런 코드를 사용하지 말고
  Class c = new String().getClass();
  // 이런 코드로 사용하자.
  Class c = String.class;
}

IdempotentOperations

멱등원 연산은 사용하지 말자. 아무련 효과가 없다.

public class Foo {
  public void bar() {
    int x = 2;
    x = x;  //멱등원(idempotent)는 아무런 효과가 없다. 
  }
}

SimpleDateFormatNeedsLocale

특정 지역읠 바탕으로 한 SimpleDateFormat의 인스턴스를 생성할 때는 Locale을 이용하여 설정해야 한다.

public class Foo {
  public void bar() {
    //다음과 같이 지역 설정을 하자.
    SimpleDateFormat sdf = new SimpleDateFormat("yyyy.MM.dd hh:mm:ss", Locale.KOREA);
    Date date = new Date();
    System.out.println(sdf.format(date));
  }
}

ImmutableField

private 접근제한자가 직접적인 선언 또는 생성자로부터 초기화된 이후로 변경되지 않는다면 final로 상수로 등록하는 것이 좋다. 또한 이는 클래스를 불변 클래스(immutable classe)로 전환하는데 도움이 된다.

public class Foo {
  private int x; // 초기화 이후 변경이 되지 않는다면 final로 설정하자.
  public Foo() {
    x = 7;
  }
  public void foo() {
    int a = x + 2;
  }
}

UseLocaleWithCaseConversions

String.toLowerCase() 또는 toUpperCase()를 호출할 경우 Locale을 사용하자. 이는 확실하게 지역적인 문제를 해결할 수 있는 방법이다. 예를들면, 터키어에서 영어로 비교할 경우 등.

void Foo {
  // 나쁜 방법
  // 이런 방식은 터키지역으로 설정된 문자열은 비교할 경우
  // 비교할 수 없다.
  if (x.toLowerCase().equals("list"))...
  /*
  아래와 같은 방식으로 문자열의 지역을 설정하고 비교해야한다
  if (x.toLowerCase(Locale.US).equals("list")) ...
  //또는 간단히 equalsIgnoreCase를 사용하자
  if (x.equalsIgnoreCase("list")) ...
  */
  // 좋은 방법
  String z = a.toLowerCase(Locale.EN);
}

AvoidProtectedFieldInFinalClass

final class 내부에서는 protected 접근제한자를 사용할 수 없다. final class는 상속될 수 없으므로 protected 선언은 의미가 없다. 명확하게 말해서 protected대신에 private 또는 package access 선언을 사용하자.

public final class Bar {
  private int x;
  // Bar는 상속될 수 없다. 그런데 y가 protected?
  protected int y;  
  Bar() {}
}

AssignmentToNonFinalStatic

안전하지 않은 static 필드 사용을 확인하자.

public class StaticField {
  //x는 static이지만 final이 아니라 변경 가능하다
  static int x;
  public FinalFields(int y) {
    //일반 메소드에서 static인 x를 사용하는 것은 안전하지 않다.
    x = y;
  }
}

MissingStaticMethodInNonInstantiatableClass

클래스가 private으로 접근 제한된 생성자만 포함하고 아무런 static 메소드 또는 필드를 갖고 있지 않다면, 이 클래스는 사용할 수 없다.

/* 이 클래스는 사용할 수 없다
 생성자가 private이므로 인스턴스화 할 수 없고
 어떠한 static 메소드나 필드가 없으므로 호출 불가능하다
 */
public class Foo {
  private Foo() {}
  void foo() {}
}

해당 URL: http://pmd.sourceforge.net/pmd-4.2.6/rules/design.html

저작자표시 비영리 변경금지

배씨네

[한글화 시리즈-07] Design Rules(1)

Design Rules(1)

UseSingleton

SimplifyBooleanReturns

SimplifyBooleanExpressions

SwitchStmtsShouldHaveDefault

AvoidDeeplyNestedIfStmts

AvoidReassigningParameters

SwitchDensity

ConstructorCallsOverridableMethod

AccessorClassGeneration

FinalFieldCouldBeStatic

CloseResource

NonStaticInitializer

DefaultLabelNotLastInSwitchStmt

NonCaseLabelInSwitchStatement

OptimizableToArrayCall

BadComparison

EqualsNull

ConfusingTernary

InstantiationToGetClass

IdempotentOperations

SimpleDateFormatNeedsLocale

ImmutableField

UseLocaleWithCaseConversions

AvoidProtectedFieldInFinalClass

AssignmentToNonFinalStatic

MissingStaticMethodInNonInstantiatableClass

티스토리툴바

[한글화 시리즈-07] Design Rules(1)

Design Rules(1)

UseSingleton

SimplifyBooleanReturns

SimplifyBooleanExpressions

SwitchStmtsShouldHaveDefault

AvoidDeeplyNestedIfStmts

AvoidReassigningParameters

SwitchDensity

ConstructorCallsOverridableMethod

AccessorClassGeneration

FinalFieldCouldBeStatic

CloseResource

NonStaticInitializer

DefaultLabelNotLastInSwitchStmt

NonCaseLabelInSwitchStatement

OptimizableToArrayCall

BadComparison

EqualsNull

ConfusingTernary

InstantiationToGetClass

IdempotentOperations

SimpleDateFormatNeedsLocale

ImmutableField

UseLocaleWithCaseConversions

AvoidProtectedFieldInFinalClass

AssignmentToNonFinalStatic

MissingStaticMethodInNonInstantiatableClass

'PMD' Related Articles

티스토리툴바