CODEWITHSUNDEEP
javaif-statementrefactoringreadability
Rnine
Rnine

Reputation: 13

Refactoring empty if-statements

I currently working on a project that I need to remove a class that is being used by different other classes. There are cases that I can remove the one line of code that consists of that class where it will never affect the functionality of the program, but also there are cases that the class that you want to be removed is inside an if-statement. The main problem is that once I removed the line of code consisting of that class where is it inside the if-statement, it will be an empty if-statement that will violates the sonar.

Is there another way to refactor an empty if-statement other that negating the condition of one of the statements? Because when I'm just negating the condition, the readability of the code reduced.

For Example:

if((example_A >= 0) && (condition_A))
{
     removeThisClass();
}
else if((example_B >= )) && (condition_B))
{
     doSomething();
}
else
{
     doAnything();
}

Refactored:

if(!((example_A >= 0) && (condition_A)) && ((example_B >= )) && (condition_B)))
{
     doSomething();
}
else
{
     doAnything();
}

Upvotes: 1

Views: 1072

Answers (3)

Thilo
Thilo

Reputation: 262534

You can put in a comment. Sonar should accept that and it could also help the reader.

void doSomething() {
  for (int i = 0; i < 42; i++)        // Non-Compliant
  {
  }
  for (int i = 0; i < 42; i++);       // Compliant

  if (myVar == 4)                     // Compliant - contains a comment
  {
    // Do nothing because of X and Y
  }
  else                                // Compliant
  {
    doSomething();
  }

  try                                 // Non-Compliant
  {
  }
  catch (Exception e)                 // Compliant
  {
    // Ignore
  }
}

Upvotes: 0

ceth
ceth

Reputation: 45295

You can put this code in separate method (https://refactoring.com/catalog/extractFunction.html) and write it like this:

public void DoSomeStuff() {

  if((example_A >= 0) && (condition_A))
    return;  

  if((example_B >= )) && (condition_B)) {
    doSomething();
    return;
  }

  doAnything();
}

Upvotes: 1

Andreas
Andreas

Reputation: 159106

If I understand you right, the line removeThisClass(); should be removed, and you don't want to be left with an empty block like this:

if((example_A >= 0) && (condition_A))
{
}
else if((example_B >= )) && (condition_B))
{
    doSomething();
}
else
{
    doAnything();
}

In order to not do the "A" tests twice, you need to negate the condition, e.g. like this:

if ((example_A < 0) || ! (condition_A))
{
    if ((example_B >= )) && (condition_B))
    {
        doSomething();
    }
    else
    {
        doAnything();
    }
}

Your refactored code is wrong, because if the "A" condition is true, the original code would execute removeThisClass();, which means it should now do nothing, but your code will call doAnything(); when "A" is true.

Upvotes: 0

Related Questions