본문 바로가기

PMD

[한글화 시리즈-21] Controversial Rules

Controversial Rules

이 룰셋은 어떤 이유이던 논란이되는 룰들을 포함한다. PMD의 모든 룰들은 custom ruleset을 통해서 적절하게 사용할 수 있으므로, 여기의 룰들도 필요에 따라서 적용하자. 이 룰셋의 룰들은 몇몇이 필요하다고 주장하지만 대부분은 좋아하지 않는 룰들을 포함한다.


UnnecessaryConstructor

이 룰은 생성자가 불필요한 경우를 경고한다. 생성자가 불필요한 경우는 클래스 내에 하나의 public 생성자외에는 어떤 한 것도 들어있지 않은 경우이다. (이거... 왜 필요하지?)

public class Foo {
  public Foo() {}
}

NullAssignment

변수를 정의한 이후 변수에 null을 할당하는 것은 일반적으로 나쁜 방법이다. 때로는 이런 배치가 프로그래머가 코드의 흐름을 정확하게 이해하지 못한다는 것을 의미하기도 한다. (하지만, null 할당의미를 알고 정확하게 사용한다면, 이런 종류의 배치는 GC(garbage collection)를 더욱 효율적으로 작동하도로 도와줄 수 있다.) - PMD는 가끔 null이 있는 모든 위치를 경고하는 경우도 있다. 조심하자.

public class Foo {
  public void bar() {
    Object x = null; // 초기화는 괜찮다.
    x = new Object();
    ... // 매우 길고 복잡한 코드
    x = null; // 코드 중간에 null로 할당하는 것은 그리 좋은 방법이 아니다.
    ... // 매우 길고 복잡한 코드
   }
}

OnlyOneReturn

메소드의 진입점과 출구는 하나여야 한다. 그리고 return으로 반환되는 위치는 메소드의 가장 마지막에 위치해야 한다.

public class OneReturnOnly1 {
  public void foo(int x) {
    if (x > 0) {
      return "hey";   // 출구는 하나만 만들자!
    }
    return "hi";
  }
}

UnusedModifier

인터페이스(interface) 내의 모든 필드들은 자동으로 public static final이며, 메소드(method)들은 모두 public abstract이다. 하나의 인터페이스 안의 클래스 또는 인터페이스들의 집합은 모두 자동적으로 public static으로 설정된다. (모든 인터페이스의 집합은 자동적으로 static이다). 역사적인 이유로, 맥락(context)을 바탕으로 자동의로 정의되는 수식어(modifier)들은 컴파일러가 알아서 받아들이므로, 꼭 명시적으로 써야할 필요가 없다.

public interface Foo {
  public abstract void bar(); // 컴파일러는 알아서 public abstract를 무시한다.
  public static final int X = 0; // public static final은 무시된다.
  public static class Bar {} // public static은 무시된다.
  public static interface Baz {} // 상동
}
public class Bar {
  public static interface Baz {} // static은 무시된다.
}

AssignmentInOperand

피연산 함수(operand) 안에서 값을 할당하는 것을 피하자. 이런 방식은 코드를 더욱 복잡하고 읽기 힘들게 만든다. 하지만 file을 읽는 프로세스에서는 빈번하게 발생하는 코드로, 신중히 적용해야한다.

public class Foo {
  public void bar() {
    int x = 2;
    //이런 식으로 조건문이나 반복문 내에서 값을 할당하는 코드는 피하자.
    if ((x = getX()) == 3) {
      System.out.println("3!");
    }
  }

  private int getX() {
    return 3;
  }
}

AtLeastOneConstructor

각 클래스는 최소한 하나의 생성자를 정의해야한다.

public class Foo {
  //최소한 하나 이상의 생성자는 꼭 정의하자!.
}

DontImportSun

sun.*에 해당하는 어떠한 package도 import하지말자. 이런 package들은 모든 시스템에 적용하 수 없고 또 변경되는 경우가 있다. 하지만 sun.misc.BASE64Encoder와 같은 매우 유용한 기능들도 sun.* 아래에 포함되어 있으므로, 신중히 결정하자

//sun.* 아래의 모든 클래스들은 신중히 사용하자.
import sun.misc.foo;
public class Foo {}

SuspiciousOctalEscape

문자열 내에 8진수형 확장특수문자(escape sequence)로 오인될만한 것들을 피하자. 자바는 section 3.10.6에서 8진수형 확장특수문자를 다음과 같은 형태로 정의한다.  

backslash  OctalDigit: \1 

backslash  OctalDigit OctalDigit: \12

backslash  ZeroToThree OctalDigit OctalDigit: \012

*OctalDigit(8진수): 0~7

*ZeroToThree: 0~3

만약 문자열 내에 "\038"과 같은 문구가 있다면 "\03"은 8진수형 확장특수문자로 변환되고 8만 정상적인 문자로 인식된다.

public class Foo {
  public void foo() {
     // \12는 8진수확장특수문자로 변환, 8'만 정상적으로 출력
     System.out.println("suspicious: \128");
  }
}

CallSuperInConstructor

아래의 방법은 생성자에서 super()를 호출하는 좋은 연습니다. 아래는 생성자에서 super()를 호출지 않고 overload된 다른 생성자 를 호출 하는 방식이다.

public class Foo extends Bar{
  public Foo() {
    // Bar의 생성자를 호출한다.
    super();
  }
  public Foo(int code) {
    ...
    //Foo의 생성자를 호출한다.
    this();
    // 이런 경우는 문제가 없다.
  }
}

UnnecessaryParentheses

불필요한 괄호(parentheses)를 사용해서 메소드를 호출하는 것처럼 보이게 하는 것을 경고한다.

public class Foo {
  boolean bar() {
    //무슨 의미가 있는 괄호일까?
    return (true);
  }
}


DefaultPackage

기본 package의 private level 대신 명확한 package 범위를 사용하자.


BooleanInversion

Boolean값을 도치(inversion)할 때 비트연산(bitwise)를 사용하자. 이게 가장 빠른 방법이다. 더욱 자세한 내용은 여기서 확인하자: http://www.javaspecialists.co.za/archive/newsletter.do?issue=042&locale=en_US

public class Foo {
  public void main(bar) {
    boolean b = true;
    b = !b; // 부정연산자는 느리다
    b ^= true; // 비트연산자는 빠르다.
  }
}

DataflowAnomalyAnalysis

데이터 흐름 상에서 지역 변수의 선언을 추적하고 분석하기 위한 룰로써 다음과 같은 타입들에 대한 룰을 담당한다. 하지만 경우에 따라 정상적인 코드도 문제점으로 지적되는 경우가 발생한다.

  1. UR: 변수의 값이 정의되지 않은 변수 (이와 같은 경우는 버그임으로 수정이 필요)
  2. DU: 기존 선언된 변수를 해제할 경우 (일반적인 소스코드에서 나타남으로 비 적용 가능)
    1. try안에서 등록된 변수는 try안에서 return 등으로 해결할 수 있도록 한다.
  3. DD: 기존 선언된 변수가 재 정의되는 경우
    1. Try 내에서 사용한 변수는 try 안에서 해결한다.


public class Foo { public void foo() { int buz = 5; buz = 6; // 위에 선언된 변수를 재 선언하는 경우 DD // 최초 변수의 null 초기화를 사용하지 않는다. foo(buz); buz = 2; int commitBatchSize = 0;//commit된 행수 int step = 0;//이경우도 DU로 PMD 룰로 경고 } // buz -> 이 메소드의 범위를 벗어날 경우 buz는 undefined됨 du-anomaly }


AvoidFinalLocalVariable

상수형 지역변수(final local variable) 사용을 피하고, 이 변수들을 클래스 맴버 변수로 바꿔라.

public class MyClass {
  public void foo() {
    //지역변수가 아닌 클래스 맴버 변수로 바꾸는 것이 좋다.
    final String finalLocalVariable;
  }
}

AvoidUsingShortType

JAVA에서 메모리 사용을 줄이기 위해서 short 형식을 사용하지만 JVM이 short에 형식에 대한 산술적 기능이 없으므로 short값을 int 형으로 강제로 변형 후 계산한다. 그러므로, 계산과 관련된 기능에서는 short형을 사용하는 것을 추천하지 않는다. 메모리의 사용량보다 이런 계산적인 부분의 문제가 더 크다. 하지만 몇몇 특정 라이브러리에서 전달 인자를 short 형으로 요구하는 경우가 종종 있다. 특히 excel과 관련된 라이브러리.

public class UsingShort {
  //short은 가능한한 사용하지 말자.
  private short doNotUseShort = 0;
  
  public UsingShort() {
    short shouldNotBeUsed = 1;
    //short으로 연산할 경우 short은 int로 형 변환을 한 후 실행하여,
    //연산 실행 속도가 느리다.
    doNotUseShort += shouldNotBeUsed;
  }
}

AvoidUsingVolatile

volatile 키워드를 사용하지 말자. 일반적으로 이 키워드는 Java 응용프로그램에서 정교한 튜닝을 위해 사용되며, 그러므로, Java의 메모리 구조(Memory Model)에 관한 전문적인 지식을 필요로한다. 이에 관한 잘못된 지식을 갖고 있을 가능성이 있다. 그러므로 volatile 키워드는 유지보수의 목저과 이식성을 위해서 사용해서는 안된다.

*volatile: Java 8.3.1.4 Language Specification 에 따르면 volatile로 필드가 선언되었을 경우, 자바 메모리 모델은 해당 변수에 대해 모든 쓰레드가 일관된 값을 사용할 수 있도록 한다. final과 volatile은 같이 쓸 수 없다.

public class ThrDeux {
  //Java 메모리 구조에 대해 잘 모르면 사용하지 말자.
  //유지보수나 이식성을 위해서도 사용하지 말자.
  private volatile String var;
}

AvoidUsingNativeCode

이미 Java는 응용프로그램을 만들기 위한  많은 도움들을 제공하고 있다. 그러므로, Java 코드가 아닌 코드에 의존해야 하는 경우는 매우 드물다. 그럼에도 불구하고 Java Naive Interface(JNI)가 드물게 사용되어야만 하는 경우도 드물게 존재한다. JNI를 사용해서 응용프로그램을 만들면 이식성이 떨어지고, 유지보수에 어려움이 따른다. 그러므로 이렇게 JNI를 사용하는 것은 추천되지 않는다.

 public class SomeJNIClass {
  public SomeJNIClass() {
    System.loadLibrary("nativelib");
  }
  
  static {
    //JNI를 불러온다 왠만하면 하지말자.
    System.loadLibrary("nativelib");
  }

  public void invalidCallsInMethod() throws SecurityException, NoSuchMethodException {
    //JNI를 불러온다 왠만하면 하지말자.
    System.loadLibrary("nativelib");
  }
}

AvoidAccessibilityAlteration

getDeclaredConstructors(), getDeclaredConstructor(Class[]) and setAccessible()과 같은 메소드와 PrivilegedAction 같은 인터페이스는 실행중에 변수와 클래스 또는 메소드가 private으로 접근 제한되어 있다고 하더라도 이런 변수, 클래스 또는 메소드의  접근 가능성을 변경할 수 있게 한다. 이는 Java의 캡슐화(encapsulation) 개념을 위반한다. 그러므로 명백하게 이런 메소드는 절대 사용해서는 안된다.

import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Method;
import java.security.PrivilegedAction;

public class Violation {
  public void invalidCallsInMethod() throws SecurityException, NoSuchMethodException {
    // getDeclaredConstructors을 호출하지말자.
    Class[] arrayOfClass = new Class[1];
    this.getClass().getDeclaredConstructors();
    this.getClass().getDeclaredConstructor(arrayOfClass);
    Class clazz = this.getClass();
    clazz.getDeclaredConstructor(arrayOfClass);
    clazz.getDeclaredConstructors();

    // setAccessible을 호출하지 말자.
    clazz.getMethod("", arrayOfClass).setAccessible(false);
    AccessibleObject.setAccessible(null, false);
    Method.setAccessible(null, false);
    Method[] methodsArray = clazz.getMethods();
    int nbMethod;
    for ( nbMethod = 0; nbMethod < methodsArray.length; nbMethod++ ) {
      methodsArray[nbMethod].setAccessible(false);
    }
    // PrivilegedAction을 사용하지 말자.
    PrivilegedAction priv = (PrivilegedAction) new Object(); priv.run();
  }
}

DoNotCallGarbageCollectionExplicitly

System.gc(), Runtime.getRuntime().gc()와 System.runFinalization()을 호출하는 것은 추천되지 않는다. 왜냐하면, 이와 같은 코드는 GC(garbage collection)의 기능과 같으며, 실행 옵션 중 -Xdisableexplicitgc 옵션으로 사용가능/불가능이 설정된다. 더욱이 최신 JVM들은 GC를 매우 잘 관리한다. 만약 메모리 사용 이슈가 메모리 누수와 관계가 없다면, JVM의 옵션들로 해결해야지 코드 자체로 해결하려 하면 안된다.

public class GCCall {
  public GCCall() {
    // 이런 명시적인 gc 호출은 금지!
    System.gc();
  }
  
  public void doSomething() {
    // 이런 명시적인 gc 호출은 금지!
    Runtime.getRuntime().gc();
  }

  public explicitGCcall() { 
    // 이런 명시적인 gc 호출은 금지!
    System.gc(); 
  }

  public void doSomething() { 
    // 이런 명시적인 gc 호출은 금지!
    Runtime.getRuntime().gc(); 
  }
}

AvoidLiteralsInIfCondition

조건문 내에서 하드코딩된 문장의 비교는 사용하지말자. 값을은 가능한한 static 변수 또는 private 클래스 맴버 변수로 변환하자.

public class PrimitiveType {
  public void downCastPrimitiveType() {
    //1은 하드코딩된 값이다. static이나 private 맴버 변수로 변경하자.
    if(i==1) {
    }
  }
}


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