2016-11-09 7 views
1

Чистый код для меня: только одна задача для каждого метода и без вложенных циклов.Как писать вложенные для циклов в методах (чистый код)

Когда я получил следующий код, я спросил себя: как я могу избежать вложенных циклов и инкапсулировать их в методах.

private String getUser(){ 
    for (FieldConfigScheme context : getConfigurationSchemes()) { 
     for (Option option : getOptions(context)) { 
      for (Group group : getGroups()) { 
       if (option.getValue().equalsIgnoreCase(group.getName())) { 
        return group.getUser(); 
       } 
      } 
     } 
    } 
    return "default"; 
} 

Моим первым решением было следующее. Проблема в том, что циклы for работают до конца и не прерываются (возвращаются), когда значение найдено и задано.

private String user = "default"; 

private String getUser(){ 
    for (FieldConfigScheme context : getConfigurationSchemes()) { 
     processOptions(context); 
    } 
    return this.user; 
} 

private void processOptions(FieldConfigScheme context){ 
    for (Option option : getOptions(context)) { 
     processGroups(option); 
    } 
} 

private void processGroups(Option option){ 
    for (Group group : getGroups()) { 
     setUser(option, group); 
    } 
} 

private void setUser(Option option, Group group){ 
    if (option.getValue().equalsIgnoreCase(group.getName())) { 
     this.user = group.getUser(); 
    } 
} 

поэтому я написал этот код, который должен быть таким же, как и первая:

private String user = "default"; 
private boolean isUserSet = false; 

private String getUser(){ 
    for (FieldConfigScheme context : getConfigurationSchemes()) { 
     if(!isUserSet) processOptions(context); 
     else return this.user; 
    } 
    return this.user; 
} 

private void processOptions(FieldConfigScheme context){ 
    for (Option option : getOptions(context)) { 
     if(!isUserSet) processGroups(option); 
     else return; 
    } 
} 

private void processGroups(Option option){ 
    for (Group group : getGroups()) { 
     if(!isUserSet) setUser(option, group); 
     else return; 
    } 
} 

private void setUser(Option option, Group group){ 
    if (option.getValue().equalsIgnoreCase(group.getName())) { 
     this.user = group.getUser(); 
     isUserSet = true; 
    } 
} 

Но потом я спросил себя, действительно ли это лучше код? Это более чистый код? Да, каждый метод только делает одно. И да, код лучше читать по моему мнению. Но из первоначально 12 строк компактного кода я теперь получил 30 строк кода и одну переменную-член больше в коде. Итак, первый исходный код лучше, потому что он более компактен даже с вложенными циклами?

Как вы думаете? Какая из них лучше? Или как я могу написать код лучше?

Заранее благодарим за ответы!

+3

Если вы используете Java 8, вы можете найти исследование лямбда, полезное для вашей цели. – Mena

+0

Как упоминалось в @Mena, возможно, что лямбда-выражения Java 8 являются одним из вариантов, но кроме этого я считаю, что ваша самая первая версия является наиболее предпочтительной. У вас есть только один метод, и вся логика содержится в нескольких вложенных циклах. –

+0

* «Чистый код означает для меня: только одно задание для каждого метода и никаких вложенных циклов». * Для меня * Чистый код * означает * читаемый, код без дублирования *. Поэтому я также предпочитаю версию «вложенного цикла». Причиной для меня является то, что в отдельных циклах нет другой работы. –

ответ

0

Вместо того, чтобы возвращать void, почему бы и нет boolean?

private String getUser(){ 
    for (FieldConfigScheme context : getConfigurationSchemes()) { 
     if (processOptions(context)) { 
      break; 
     } 
    } 
    return this.user; 
} 

private boolean processOptions(FieldConfigScheme context){ 
    for (Option option : getOptions(context)) { 
     if (processGroups(option)) { 
      return true; 
     } 
    } 
    return false; 
} 

private boolean processGroups(Option option){ 
    for (Group group : getGroups()) { 
     if (option.getValue().equalsIgnoreCase(group.getName())) { 
      this.user = group.getUser(); 
      return true; 
     } 
    } 
    return false; 
} 

T.B.H. Я предпочитаю метод вложенных циклов. Он выглядит чистым, в цикле больше ничего не происходит, чем просто найти что-то в иерархии, и это прекрасно.

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

Также этот метод, как представляется, не принимает никаких параметров, которые предполагают, что на самом деле это нужно только один раз, и в остальное время он должен просто вернуть одно и то же значение. Это просто предположение, но если это так, то это делает ваши усилия, чтобы сделать его более чистым, тем более ненужным.