Category: Back-end

sonarcube는
지속적으로 코드의 품질을 높이고 유지 보수하는데 도움을 주는 정적 분석 애플리케이션으로 분석 기능이 탁월하고 개선 사항도 자세하게 안내해 준다.
간단하게는 코딩 컨벤션부터 코드의 잠재적인 위험 사항까지 안내해주어 올바른 코딩 습관과 코드의 품질향상에 많은 도움을 준다.

다운로드 및 설치
https://www.sonarqube.org/downloads/ 에서 다운로드 받고 압축을 해제.

issue 1. 

Either log or rethrow this exception (5)

 – cause : When handling a caught exception, the original exception’s message and stack trace should be logged or passed forward.

// Noncompliant - exception is lost 

try { 

  /* ... */

} catch (Exception e) {

  LOGGER.info("context");

}


// Noncompliant - exception is lost (only message is preserved)

try { 

  /* ... */

} catch (Exception e) {

  LOGGER.info(e.getMessage());

}


// Noncompliant - exception is lost

try { 

  /* ... */

} catch (Exception e) {

  throw new RuntimeException("context"); 

}

try { 
 
  /* ... */ 

} catch (Exception e) { 

  LOGGER.info(e); 

}


try { 

  /* ... */

} catch (Exception e) {

 throw new RuntimeException(e); 

}



try {

  /* ... */

} catch (RuntimeException e) {

  doSomething();

  throw e;

} catch (Exception e) {

  // Conversion into unchecked exception is also allowed

  throw new RuntimeException(e);

}

issue 2. 

Define and throw a dedicated exception instead of using a generic one (7)

 – cause : Using such generic exceptions as Error, RuntimeException, Throwable, and Exception prevents calling methods from handling true, system-generated exceptions differently than application-generated errors.

public void foo(String bar) throws Throwable {  // Noncompliant 

  throw new RuntimeException("My Message");     // Noncompliant 

}

public void foo(String bar) { 

 throw new MyOwnRuntimeException("My Message"); 

}

issue 3. 

Use isEmpty() to check whether the collection is empty or not. (1)

 – cause : Using Collection.size() to test for emptiness works, but using Collection.isEmpty() makes the code more readable and can be more performant. The time complexity of any isEmpty() method implementation should be O(1) whereas some implementations of size() can be O(n).

if (myCollection.size() == 0) {  // Noncompliant 

  /* ... */ 

}

if (myCollection.isEmpty()) { 

  /* ... */ 

}

issue 4.

Move the “update” string literal on the left side of this string comparison.

String myString = null;


System.out.println("Equal? " + myString.equals("foo"));

// Noncompliant; will raise a NPE


System.out.println("Equal? " + (myString != null && myString.equals("foo")));  

// Noncompliant; null check could be removed

System.out.println("Equal?" + "foo".equals(myString)); 
                       // properly deals with the null case 

issue 5.

Merge this if statement with the enclosing one. (4)

 – cause : Merging collapsible if statements increases the code’s readability.

if (file != null) { 

  if (file.isFile() || file.isDirectory()) { 

    /* ... */ 

  } 

}

if (file != null && isFileOrDirectory(file)) { 

  /* ... */ 

} 

private static boolean isFileOrDirectory(File file) { 

  return file.isFile() || file.isDirectory();

}

issue 6.

Remove this unused “변수명”  (33)

 – cause : If a private field is declared but not used in the program, it can be considered dead code and should therefore be removed. This will improve maintainability because developers will not wonder what the variable is used for. (주로 도메인 객체)

public class MyClass {

  private int foo = 42;



  public int compute(int a) {

    return a * 42;

  }

}

public class MyClass {

   public int compute(int a) { 

      return a * 42;

   }

}

issue 7. 

The Cyclomatic Complexity of this method 메소드명. (9)

 – cause : The cyclomatic complexity of methods should not exceed a defined threshold. 
                  Complex code can perform poorly and will in any case be difficult to understand and therefore to maintain.

-

-

issue 8.

duplicated blocks of code must be removed. (3)

 – cause : An issue is created on a file as soon as there is at least one block of duplicated code on this file.

-

-

issue 9.

The type of the “input” object should be an interface such as “Map” rather than the implementation “HashMap”. (2)

 – cause : The purpose of the Java Collections API is to provide a well defined hierarchy of interfaces in order to hide implementation details.

  Implementing classes must be used to instantiate new collections, but the result of an instantiation should ideally be stored in a variable whose type is a Java Collection interface.

  This rule raises an issue when an implementation class:

• is returned from a public method.
• is accepted as an argument to a public method.
• is exposed as a public member.

public class Employees { 

  private HashSet<Employee> employees = new HashSet<Employee>();  
// Noncompliant - "employees" should have type "Set" rather than "HashSet"



  public HashSet<Employee> getEmployees() {  // Noncompliant 

    return employees;

  }

}

public class Employees {  

 private Set employees = new HashSet();  // Compliant

public Set getEmployees() { // Compliant 
     
return employees;

   }

 }

issue 10.

Reduce the number of conditional operators {:num} used in the expression (maximum allowed :num). (3)

 – cause : The complexity of an expression is defined by the number of &&, || and condition ? ifTrue : ifFalse operators it contains. 
                   A single expression’s complexity should not become too high to keep the code readable.

With the default threshold value of 3: 

if (((condition1 && condition2) || (condition3 && condition4)) && condition5) {

 ... 

}

if ( (myFirstCondition() || mySecondCondition()) && myLastCondition()) {
 ... 
}

issue 11.

Method has {:num} parameters, which is greater than {:num} authorized. (3)

 – cause : A long parameter list can indicate that a new structure should be created to wrap the numerous parameters or that the function is doing too many things.

With a maximum number of 4 parameters:

public void doSomething(int param1, int param2, int param3, String param4, long param5) {

 ...

}

public void doSomething(int param1, int param2, int param3, String param4) {

 ... 

}

issue 12.

Refactor this code to not nest more than 3 if/for/while/switch/try statements. (3)

 – cause : Nested if, for, while, switch, and try statements is a key ingredient for making what’s known as “Spaghetti code”. Such code is hard to read, refactor and therefore maintain.

With the default threshold of 3: 

if (condition1) { // Compliant - depth = 1  
 
  /* ... */   

  if (condition2) { // Compliant - depth = 2  

    /* ... */   

    for(int i = 0; i < 10; i++) {  // Compliant - depth = 3, not exceeding the limit 

       /* ... */ 
       
       if (condition4) {  // Noncompliant - depth = 4  
       
          if (condition5) {  // Depth = 5, exceeding the limit, but issues are only reported on depth = 4 
        
  /* ... */  
      
} 

 return; 
      
} 
 } 
 } 
}

-

issue 13.

Complete the task associated to this TODO comment.  (2)

 – cause : TODO tags are commonly used to mark places where some more code is required, but which the developer wants to implement later.
Sometimes the developer will not have the time or will simply forget to get back to that tag. 
This rule is meant to track those tags, and ensure that they do not go unnoticed.

void doSomething() {   // TODO }

-

issue 14. issue 15. issue 16. issue 17.

Document this public class.  (17)
Document the parameter(s): page. (16)
Document this public interface. (5)
Document this public method. (70)
Document this public enum. (5)
Document this public constructor. (5) 

– cause : Try to imagine using the standard Java API (Collections, JDBC, IO, …) without Javadoc. It would be a nightmare, because Javadoc is the only way to understand of the contract of the API. Documenting an API with Javadoc increases the productivity of the developers consuming it.

The following Javadoc elements are required:

• Parameters, using @param parameterName.
• Method return values, using @return.
• Generic types, using @param <T>.

The following public methods and constructors are not taken into account by this rule:

• Getters and setters.
• Methods with @Override annotation.
• Empty constructors.
• Static constants.

/**

 * This is a Javadoc comment

 */

public class MyClass implements Runnable { // Noncompliant - missing '@param '  

 public static final DEFAULT_STATUS = 0; // Compliant - static constant

 private int status;     // Compliant - not public 

 public String message;  // Noncompliant   

 public MyClass() {      // Noncompliant - missing documentation 
  
   this.status = DEFAULT_STATUS;

 }



 public void setStatus(int status) {  // Compliant - setter  

   this.status = status; 

 }

   

@Override

public void run() {  // Compliant - has @Override annotation   } 
  protected void doSomething() {    // Compliant - not public   } 
  public void doSomething2(int value) {  // Noncompliant   } 

public int doSomething3(int value) {  // Noncompliant  

   return value;

}
 
}

/** 

 * This is a Javadoc comment   

 * @param  ...   

 */

public class MyClass implements Runnable {

   public static final DEFAULT_STATUS = 0;

   private int status;

   /**

     * This is a Javadoc comment

     */

   public String message;


   /**

    * Class comment...

    */

   public MyClass() {

     this.status = DEFAULT_STATUS;

   }

   public void setStatus(int status) { 

    this.status = status;

   }


   @Override

   public void run() {   }

   protected void doSomething() {   }

   /**

     * @param value ... 
    
     */

   public void doSomething(int value) {

   /**

     *  {@inheritDoc}

     */

   public int doSomething(int value) {

     return value;

   }

 }

issue 18.

Immediately return this expression instead of assigning it to the temporary variable (5)

– cause : Declaring a variable only to immediately return or throw it is a bad practice.
Some developers argue that the practice improves code readability, because it enables them to explicitly name what is being returned. However, this variable is an internal implementation detail that is not exposed to the callers of the method. The method name should be sufficient for callers to know exactly what will be returned.

public long computeDurationInMilliseconds() {

   long duration = (((hours * 60) + minutes) * 60 + seconds ) * 1000;
  return duration;

}


public void doSomething() {

   RuntimeException myException = new RuntimeException();

   throw myException; 

}

public long computeDurationInMilliseconds() {

   return (((hours * 60) + minutes) * 60 + seconds ) * 1000 ; 

}


public void doSomething() {

   throw new RuntimeException(); 

}

issue 19.

Define a constant instead of duplicating this literal “{:variable name}” {:num} times. (9)

– cause : Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.
On the other hand, constants can be referenced from many places, but only need to be updated in a single place.

public void run() { 

  prepare("action1"); // Noncompliant - "action1" is duplicated 3 times 
execute("action1");
  release("action1"); 

}

@SuppressWarning("all") // Compliant - annotations are excluded
 private void method1() {

 /* ... */

}


@SuppressWarning("all") 

private void method2() {

 /* ... */

}


public String method3(String a) {

   System.out.println("'" + a + "'"); // Compliant - literal "'" has less than 5 characters and is excluded

   return ""; // Compliant - literal "" has less than 5 characters and is excluded 

}

private static final String ACTION_1 = "action1";  // Compliant 

public void run() { 

  prepare(ACTION_1);  // Compliant 

  execute(ACTION_1);  

  release(ACTION_1); 

}

issue 20.

At most one statement is allowed per line, but {:num} statements were found on this line. (3)

– cause : For better readability, do not put more than one statement on a single line.

if(someCondition) doSomething();

if(someCondition) {   doSomething(); }

issue 21.

Reorder the modifiers to comply with the Java Language Specification.  (22)

– cause : The Java Language Specification recommends listing modifiers in the following order:

1. Annotations
2. public
3. protected
4. private
5. abstract
6. static
7. final
8. transient
9. volatile
10. synchronized
11. native
12. strictfp
Not following this convention has no technical impact, but will reduce the code’s readability because most developers are used to the standard order.

static public void main(String[] args) {   // Noncompliant }

public static void main(String[] args) {   // Compliant }