2016-10-03 3 views
0

Как удалить ниже глубоко вложенное выражение if.Глубоко вложенный оператор if удалить PMD

List<c1> list1 = new ArrayList(); 

    } 
} 
+0

Для будущего: этот вопрос «как я могу улучшить этот код» будет лучше (и, вероятно, получить ответы более высокого качества!) На http://codereview.stackexchange.com/. Stackoverflow больше подходит для технических проблем, а не для улучшения кода. –

ответ

2

Переведите код в новые методы. Для петли хороший кандидат. После того, как он в своем собственном методе, вы можете сделать это проще, тоже: по возвращении abc как только вы найдете матч:

private String find(List<c1> list1) { 
     for (Coverage abc :list1) { 
      if(null != abc){ 
       if(("eu".equalsIgnoreCase(abc.getCoverageCd())) || ("mn".equalsIgnoreCase(abc.getCoverageCd()))){ 
        return abc; 
       } 
      } 
     } 
     return null; 
} 

Тогда просто вызовите его из существующего кода:

list1 = listthree.getCoverageList(); 
myCoverage = find(list1); 

также удалили проверку if(!list1.isEmpty()){ - это не нужно. Если список пуст, for-loop будет в порядке: он просто ничего не сделает.

Последнее, что вы можете сделать, это убедиться, что вам действительно нужны все эти null чеки. Если эти значения никогда не могут быть нулевыми, то не добавляйте нулевую проверку.

+0

без создания метода, как я могу исправить эту проблему ?? – susho90

3

Я думаю, что ваша настоящая проблема заключается в том, что вы/ваше приложение чрезмерно используете null, либо намеренно, либо потому, что у вас много проблем с неинициализированными переменными и массивами.

Если вы отследили и устранили источник этих нулей, вы можете избавиться от всех тех нулевых проверок, которые делают ваш код сложным. (На самом деле у меня возникло бы желание удалить чеки в любом случае и использовать полученные NPE, чтобы помочь вам отследить, откуда берутся значения null.)

Чтобы проиллюстрировать это, будет выглядеть ваш код, если там не было null s беспокоиться о:

Asset listthree = (Asset) getPIFObject(aModelManager, list2); 
List<c1> list1 = listthree.getCoverageList(); 
for (Coverage abc : list1) { 
    if (("eu".equalsIgnoreCase(abc.getCoverageCd())) || 
     ("mn".equalsIgnoreCase(abc.getCoverageCd()))) { 
     myCoverage = abc; 
     break; 
    } 
} 

(Я также удалил ненужный тест, чтобы увидеть, если list1 пуст, и переместил декларацию list1.)

1

Как сказал @StephenC, лучший вариант заключается в том, чтобы избежать предоставления null v чтобы вы могли удалить эти нулевые проверки. Однако, если по каким-то причинам вы не можете быть уверены, что никакие null значения не предусмотрены, и вы должны держать эти пустые чеки вы можете реорганизовывать вам код с помощью Optional:

return Optional.ofNullable(list2) 
     .map(f -> getPIFObject(aModelManager, list2)) 
     .map(a -> a.getCoverageList().stream()) 
     .orElse(Stream.empty()) 
     .filter(c -> "eu".equalsIgnoreCase(c.getCoverageCd()) || "mn".equalsIgnoreCase(c.getCoverageCd())) 
     .findFirst() 
     .orElseThrow(() -> new CoverageException("No coverage found !")); //or orElse(defaultValue); 

Если вы абонент этого метода, затем обеспечить, чтобы никогда не пройти null (вместо давать пустые списки/значения по умолчанию/нулевые объекты), то ваш код может быть значительно упрощено до:

Asset listthree = (Asset) getPIFObject(aModelManager, list2); 
for (Coverage abc : listthree.getCoverageList()) { 
    if (("eu".equalsIgnoreCase(abc.getCoverageCd())) || ("mn".equalsIgnoreCase(abc.getCoverageCd()))) { 
     return abc; 
    } 
} 
throw new CoverageException("No coverage found !"); //or return a default value 

Это достигается такое же поведение, как описано выше, но с Optional:

Asset listthree = (Asset) getPIFObject(aModelManager, list2); 
return listthree.getCoverageList().stream() 
     .filter(c -> "eu".equalsIgnoreCase(c.getCoverageCd()) || "mn".equalsIgnoreCase(c.getCoverageCd())) 
     .findFirst() 
     .orElseThrow(() -> new CoverageException("No coverage found !")); //or orElse(defaultValue); 
Смежные вопросы