Чистый код - я должен изменить литерал 1 на константу?

Чтобы избежать магических чисел, мы часто слышим, что мы должны дать литералу осмысленное имя. Такие как:

//THIS CODE COMES FROM THE CLEAN CODE BOOK
for (int j = 0; j < 34; j++) {
    s += (t[j] * 4) / 5;
}

-------------------- Change to --------------------

int realDaysPerIdealDay = 4;
const int WORK_DAYS_PER_WEEK = 5;
int sum = 0;
for (int j = 0; j < NUMBER_OF_TASKS; j++) {
    int realTaskDays = taskEstimate[j] * realDaysPerIdealDay;
    int realTaskWeeks = (realdays / WORK_DAYS_PER_WEEK);
    sum += realTaskWeeks;
}

У меня есть фиктивный метод, как это:

Объясните: я предполагаю, что у меня есть список людей, которых нужно обслуживать, и по умолчанию мы тратим 5 долларов на покупку только еды, но когда у нас больше одного человека, нам нужно покупать воду и еду, мы должны тратить больше денег, может быть, 6 долларов. Я изменю свой код, пожалуйста , обратите внимание на литерал 1 , мой вопрос об этом.

public int getMoneyByPersons(){
    if(persons.size() == 1){ 
        // TODO - return money for one person
    } else {
        // TODO - calculate and return money for people.
    }

}

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

Итак, мой вопрос должен ли я дать имя для буквального значения 1? Когда значение является магическим числом, а когда нет? Как я могу различить контекст, чтобы выбрать лучшее решение?

Нет. В этом примере 1 совершенно значимо.

Однако что, если Person.size () равен нулю? Кажется странным, что persons.getMoney()работает для 0 и 2, но не для 1.

Почему кусок кода содержит это конкретное буквальное значение?

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

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

• В первом примере, значение литералов , таких как 34, 4, 5не очевидно из контекста. Вместо этого некоторые из этих значений имеют особое значение в вашей проблемной области. Поэтому было хорошо дать им имена.

• Во втором примере значение литерала 1очень ясно из контекста. Введение имени не полезно.

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

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

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

Для «очевидных» литералов нет ограничения по размеру, потому что это полностью зависит от контекста. Например, литерал 1024может быть совершенно очевиден в контексте вычисления размера файла, или литерал 31в контексте хеш-функции, или литерал padding: 0.5emв контексте таблицы стилей CSS.

У этого куска кода есть несколько проблем, которые, кстати, могут быть сокращены следующим образом:

public List<Money> getMoneyByPersons() {
    return persons.size() == 1 ?
        moneyService.getMoneyIfHasOnePerson() :
        moneyService.getMoney(); 
}

1. Непонятно, почему один человек является особым случаем. Я предполагаю, что есть определенное бизнес-правило, которое говорит, что получение денег от одного человека радикально отличается от получения денег от нескольких человек. Тем не менее, я должен пойти и посмотреть внутрь обоих, getMoneyIfHasOnePersonи getMoney, в надежде понять, почему существуют разные случаи.

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

3. Есть ли причина возвращать List<Money>коллекцию, а не коллекцию?

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

Как я могу различить контекст, чтобы выбрать лучшее решение?

Вы делаете все, что делает ваш код более явным.

Пример 1

Представьте себе следующий фрагмент кода:

if (sequence.size() == 0) {
    return null;
}

return this.processSequence(sequence);

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

if (sequence.isEmpty()) {
    return null;
}

return this.processSequence(sequence);

Здесь больше нет констант, а код еще понятнее.

Пример 2

Возьмите другой кусок кода:

const result = Math.round(input * 1000) / 1000;

Не требуется много времени, чтобы понять, что он делает в таких языках, как JavaScript, которые не round(value, precision)перегружены.

Теперь, если вы хотите ввести константу, как бы она называлась? Ближайший срок вы можете получить Precision. Так:

const precision = 1000;
const result = Math.round(input * precision) / precision;

Это улучшает читабельность? Это может быть. Здесь значение константы довольно ограничено, и вы можете спросить себя, действительно ли вам нужно выполнить рефакторинг. Приятно то, что теперь точность объявляется только один раз, поэтому, если она изменится, вы не рискуете ошибиться, например:

const result = Math.round(input * 100) / 1000;

изменить значение в одном месте и забыть сделать это в другом.

Пример 3

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

Возьмите следующий кусок кода:

class Point
{
    ...
    public void Reset()
    {
        x, y = (0, 0);
    }
}

Если вы попытаетесь заменить нули на переменную, сложность будет найти значимое имя. Как бы вы назвали это? ZeroPosition? Base? Default? Введение здесь константы никак не улучшит код. Это сделало бы это немного длиннее, и только это.

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

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

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

Я думаю о том,

// Just an example name  
function normalize_task_value(task) {  
    return (task * 4) / 5;  // or to `return (task * 4) / NUMBER_OF_TASKS` but it really matters what logic you want to convey and there are reasons to many ways to make this logical. A comment would be the greatest improvement

}  

// Is it possible to just use tasks.length or something like that?  
// this NUMBER_OF_TASKS is the only one thats actually tricky to   
// understand right now how it plays its role.  

function normalize_all_task_values(tasks, accumulator) {  
    for (int i = 0; i < NUMBER_OF_TASKS; i++) {  
        accumulator += normalize_task_value(tasks[i]);
    }
}  

Извините, если я не в базе, но я просто разработчик javascript. Я не уверен, какую композицию я обосновал этим, я думаю, что не все должно быть в массиве или списке, но длина имеет много смысла. И * 4 будет хорошей константой, поскольку ее происхождение туманно.

Это число 1, это может быть другое число? Может ли это быть 2 или 3, или есть логические причины, почему это должно быть 1? Если это должно быть 1, то использование 1 хорошо. В противном случае вы можете определить константу. Не называйте это константой ОДИН. (Я видел, что сделано).

60 секунд в минуту - вам нужна постоянная? Ну, это 60 секунд, а не 50 или 70. И все это знают. Так что это может остаться рядом.

60 страниц, напечатанных на странице - это число могло бы легко составить 59, 55 или 70. На самом деле, если вы измените размер шрифта, оно может составить 55 или 70. Так что здесь важнее константа, которая требуется больше.

Это также вопрос, насколько ясен смысл. Если вы напишите «минуты = секунды / 60», это понятно. Если вы напишите «x = y / 60», это не ясно. Где-то должны быть какие-то значимые имена.

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

Я видел достаточное количество кода, как в (модифицированном) OP в поисках БД. Запрос возвращает список, но бизнес-правила говорят, что может быть только один элемент. И затем, конечно, что-то изменилось для «только этого одного случая» в список с более чем одним элементом. (Да, я сказал довольно много раз. Это почти как они ... нм)

Поэтому вместо создания константы я бы (в методе чистого кода) создал метод, чтобы дать имя или прояснить, что условное выражение предназначено для обнаружения (и инкапсулировать, как оно его обнаруживает):

public int getMoneyByPersons(){
  if(isSingleDepartmentHead()){ 
    // TODO - return money for one person
  } else {
    // TODO - calculate and return money for people.
  }
}

public boolean isSingleDepartmentHead() {
   return persons.size() == 1;
}