Должен ли я предпочитаю читаемость по безопасности в моем коде?

Question

Я имею дело с каким-то кодом, который имеет последовательную структуру создания экземпляров Карты, как это:

Map<String, String> valuesMap = new HashMap<String, String>(); 
valuesMap.put("UserName", "Value1"); 
valuesMap.put("FirstName", "Value2"); 
valuesMap.put("LastName", "Value3"); 
valuesMap.put("Email", "Value4"); 

Я думал, что это может быть читаемым, как это:

Map<String, String> valuesMap = createMap(
    "UserName", "Value1", 
    "FirstName", "Value2", 
    "LastName", "Value3", 
    "Email", "Value4" 
);    

С помощью следующего метода:

private Map<String, String> createMap(String... parameters) { 
    Map<String, String> valueMap = new HashMap<String, String>(); 
    if (parameters.length % 2 == 0) { 
     for (int i = 0; i < parameters.length; i+=2){ 
      String key = parameters[i]; 
      String value = parameters[i + 1]; 
      valueMap.put(key, value); 
     } 
    } else { 
     throw new IllegalArgumentException("The parameters to this method must be even in number"); 
    } 
    return valueMap; 
} 

Но при этом я теряю способность ловить ошибки во время компиляции. Например: кто-то может легко сделать следующий

Map<String, String> valuesMap = createMap(
    "UserName", "Value1", 
    "FirstName", "Value2", 
    "LastName", // missing value for key 
    "Email", "Value4" 
);    

Я соблазном использовать более читаемый метод, нужны предложения от вас, ребят.

Edit:

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

---- Скит @ Джон Спасибо за указание на ошибку.

Answer 1

Метод, который принимает длинный список параметров одного и того же типа, считается плохой практикой.

В вашем случае вместо использования Карты, содержащей данные для вашего пользователя. Подумайте о создании пользователя класса с членами userName, firstName, lastName и электронной почтой.

Answer 2

Несмотря на различные комментарии, я видел, что вспомогательные методы, подобные этому, использовались совсем немного, для статической инициализации, и обычно они были проще, чем альтернативы. Конечно, он работает только тогда, когда ваш ключ и тип значения одинаковы - в противном случае вы могли бы обеспечить перегрузки для (скажем) до 4 ключей и 4 значений, после чего вы также получите безопасность во время компиляции.

Вы фактически не пропускаете безопасность - вы откладываете безопасность времени компиляции до безопасности выполнения. До тех пор, пока у вас есть хотя бы один модульный тест, который выполняет этот код, вы обнаружите ошибку (как вы выбрали исключение). По моему опыту, такие карты: обычно используются либо в тестах, либо как статические окончательные карты, которые никогда не мутируются. (В последнем случае, рекомендуется использовать неизменяемый карту для обеспечения отсутствия мутации ...)

То есть, я бы реорганизовать метод просто проверить счет первого:

if (parameters.length % 2 != 0) { 
    throw new IllegalArgumentException("Odd number of keys and values provided"); 
} 
// Now we can just proceed 
Map<String, String> valueMap = new HashMap<String, String>(); 
for (int i = 0; i < parameters.length; i+=2) { 
    String key = parameters[i]; 
    String value = parameters[i + 1]; 
    valueMap.put(key, value); 
} 
return valueMap; 

Заметьте, что вы перед ошибкой в ​​вашем демографическом коде произошла ошибка из-за проверки границ вашего цикла.

Answer 3

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

Если вы можете определить имя и «значение», вы должны это сделать.

Я делаю это совсем немного - просто помните, что всякий раз, когда вы удаляете проверку времени компиляции с таким кодом, вы принимаете на себя ответственность за выполнение во время выполнения!

Кстати, вы также можете улучшить как проверку ошибок, так и читаемость (запятая используется только как предложение, вы просто выбираете какой-то разделитель, который не может идти ни в одном поле, запятой, двоеточием и тильди - это хороший выбор):

String[] values=new String[]{ 
    "Name1,Value1", 
    "Name2,Value2", 
    ... 
} 

Then to parse: 

for(String s:values) { 
    String[] splitted=s.split(",") 
    assert(splitted.size == 2) 
    valuesMap.put(splitted[0],splitted[1]) 
} 

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

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

Answer 4

Я призываю вас рассмотреть возможность сделать это следующим образом:

@SafeVarargs 
public static <K,V> HashMap<K,V> hashMap(Map.Entry<K,V>... kvs) { 
    return map(HashMap::new, kvs); 
} 

@SafeVarargs 
public static <M extends Map<K, V>, K, V> M map(
    Supplier<? extends M> supplier, Map.Entry<K, V>... kvs) 
{ 
    M m = supplier.get(); 
    for (Map.Entry<K,V> kv : kvs) m.put(kv.getKey(), kv.getValue()); 
    return m; 
} 

public static <K,V> Map.Entry<K,V> kv(K k, V v) { 
    return new SimpleImmutableEntry<K, V>(k,v); 
} 

Ваш клиентский код будет выглядеть следующим образом:

Map<Integer, String> map = hashMap(kv(1, "a"), kv(2, "b")); 

и было бы 100% типизированного, потому что пары ключ-значение представленный на уровне статического типа.

Кстати, это не зависит в основном от особенностей Java 8. Вот упрощенная версия для Java 7:

@SafeVarargs 
public static <K,V> HashMap<K,V> hashMap(Map.Entry<K,V>... kvs) { 
    HashMap<K, V> m = new HashMap<>(); 
    for (Map.Entry<K, V> kv : kvs) m.put(kv.getKey(), kv.getValue()); 
    return m; 
} 

Answer 5

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

Здесь нет конфликта. Все, что вы сделаете, чтобы сделать реализацию вспомогательного метода более безопасным, сделает его использование более читаемым.

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

"Readable" означает, что вам не нужно тщательно изучать шаблон каждый раз, когда вы его видите, не так ли?