Очистить код. Должен ли я изменить переменную 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? Когда значение является магическим числом, а когда нет? Как я могу отличить контекст, чтобы выбрать лучшее решение?

6 голосов | спросил Jacky 11 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowTue, 11 Sep 2018 06:16:42 +0300 2018, 06:16:42

5 ответов


16

Нет. В этом примере 1 имеет смысл.

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

ответил user949300 11 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowTue, 11 Sep 2018 08:20:40 +0300 2018, 08:20:40
11

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

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

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

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

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

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

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

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

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

ответил amon 11 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowTue, 11 Sep 2018 14:29:29 +0300 2018, 14:29:29
5

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

 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? Представление константы здесь никак не улучшит код. Это сделало бы его немного дольше, и только это.

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

ответил Arseni Mourzenko 11 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowTue, 11 Sep 2018 14:35:49 +0300 2018, 14:35:49
2

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

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

Я думаю.

// Just an example name  
function normalize_task_value(task) {  
    return (task * 4) / 5;  
}  

// 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.

ответил etisdew 11 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowTue, 11 Sep 2018 07:26:58 +0300 2018, 07:26:58
0

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

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

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

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

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

ответил gnasher729 12 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowWed, 12 Sep 2018 02:18:56 +0300 2018, 02:18:56

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132