Это хорошая практика, чтобы использовать функции только для централизации общего кода?

20

Я часто сталкиваюсь с этой проблемой. Например, в настоящее время я пишу функцию чтения и функцию записи, и они оба проверяют, bufявляется ли указатель NULL, и находится ли modeпеременная в определенных границах.

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

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

EpsilonVector
источник
2
Однострочный материал требует не только использования в нескольких местах, чтобы гарантировать переход в отдельную функцию.
1
Отличный вопрос Там было так много раз, я удивлялся тому же самому.
rdasxy

Ответы:

31

Это отличное использование функций.

Это будет довольно анемичная функция (мало что делает) ...

Это хорошо. Функции должны делать только одно.

скорее локализовано ...

На языке ОО сделайте его приватным.

(так не общего назначения)

Если он обрабатывает более одного случая, он имеет общее назначение. Более того, обобщение - это не единственное использование функций. Они действительно существуют для того, чтобы (1) сэкономить на написании одного и того же кода более одного раза, а также (2) для разбивки кода на более мелкие куски, чтобы сделать его более читабельным. В этом случае он достигает как (1), так и (2). Однако, даже если ваша функция вызывается из одного места, она все равно может помочь с (2).

и сам по себе плохо себя чувствует (не может понять, для чего он вам нужен, если вы не видите, где он используется).

Придумайте хорошее имя для него, и оно само по себе прекрасно. "ValidateFileParameters" или что-то. Теперь стоит нормально самостоятельно.

Kramii Восстановить Монику
источник
6
Хорошо товарные точки. Я бы добавил (3), чтобы вы не искали дублированную ошибку во всей кодовой базе, когда вы можете исправить в одном месте. Дублирование кода может очень быстро привести к черту обслуживания.
Deadalnix
2
@deadalnix: Хороший вопрос. Когда я писал, я понял, что мои пункты были чрезмерным упрощением. Написание и более простая отладка - это, безусловно, преимущество разделения функций на функции (как и возможность модульного тестирования отдельных функций).
Kramii восстановит Монику
11

Это так полностью должно быть функцией.

if (isBufferValid(buffer)) {
    // ...
}

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

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

Позвольте мне задать вам лучший вопрос. Как это не хорошая практика?

Делать правильные вещи. :)

Ям Маркович
источник
4
Если isBufferValid просто, return buffer != null;то я думаю, что вы ухудшаете читабельность там.
фунтовые
5
@pdr: В этом простом случае это ухудшает читабельность, если вы настроены на уродство управления и действительно, действительно, ДЕЙСТВИТЕЛЬНО хотите знать, как код проверяет правильность буфера. Так что это субъективно в этих простых случаях.
Спойк
4
@ pdr Я не знаю, как это ухудшает читабельность по любым стандартам. Вы освобождаете других разработчиков от заботы о том, как вы делаете что-то, и сосредоточенности на том, что вы делаете. isBufferValidопределенно более читабельным (в моей книге), чем buffer != null, потому что он сообщает цель более четко. И опять же, не говоря уже о том, что это спасает вас от дублирования и здесь. Что еще тебе нужно?
Ям Маркович
5

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

Конечно, существуют ограничения, продиктованные здравым смыслом. Например, вы не хотите иметь WriteToConsole(text)метод с простым телом Console.WriteLine(text). Но заблуждение в сторону читабельности - хорошая практика.

Konamiman
источник
2

Обычно рекомендуется использовать функции для удаления дублирования в коде.

Однако это может быть слишком далеко. Это суждение.

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

if (buf==null) throw new BufferException("Null read buffer!");

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

checkForNullBuffer(buf, "Null read buffer!");

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

mikera
источник
Возможно, ваш пример говорит больше об отсутствии контрактной поддержки в языке, чем о реальной необходимости дублирования. Но хорошие моменты в любом случае.
Deadalnix
1
Ты как-то упустил из виду то, что я вижу. Мне не нужно переступать или учитывать логику каждый раз, когда вы отлаживаете . Если я хочу знать, как это делается, я проверяю это один раз. Делать это каждый раз просто глупо.
Ям Маркович
Если сообщение об ошибке («Нулевой буфер чтения») повторяется, это дублирование обязательно должно быть устранено.
Кевин Клайн
Ну, это только пример, но, вероятно, вы будете иметь разные сообщения на каждом сайте вызова функций - например, «Пустой буфер чтения» и «Пустой буфер записи», например. Если вам не нужны одни и те же журналы / сообщения об ошибках для каждой ситуации, вы не сможете их дублировать .......
mikera
-1 Preconditions.checkNotNull - это хорошая практика, а не плохая. Не нужно включать строковое сообщение, хотя. google-collections.googlecode.com/svn/trunk/javadoc/com/google/…
ripper234
2

Централизация кода, как правило, всегда хорошая идея. Мы должны повторно использовать код как можно больше.

Однако важно отметить, как это сделать. Например, если у вас есть код, который выполняет compute_prime_number () или check_if_packet_is_bad (), это хорошо. Скорее всего, сам алгоритм функциональности будет развиваться, что будет выгодно.

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

Вот некоторые вопросы, которые вы должны задать, прежде чем задавать

  1. Имеет ли функция, которую вы создаете, свое собственное значение или это просто набор строк?

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

  3. Каковы будут ожидания (различных частей) приложений при создании исключений?

  4. Каковы сценарии, чтобы увидеть, что функции будут развиваться?

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

По сути, вопрос таков: «Действительно ли эта новая функция заслуживает повторного использования или это просто копирование-вставка ?» Если это первое, хорошо идти.

Дипан Мехта
источник
1
Ну, если дублированный код не имеет своего собственного значения, ваш код говорит вам, что он нуждается в рефакторинге. Потому что места, где происходит дублирование, вероятно, также не имеют своего собственного значения.
Deadalnix
1

Дублирования кода следует избегать. Каждый раз, когда вы ожидаете это, вы должны избегать дублирования кода. Если вы этого не ожидали, примените правило 3: рефакторинг до того, как один и тот же фрагмент кода будет продублирован 3 раза, аннотируйте причуду при его дублировании 2 раза.

Что такое дублирование кода?

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

Рассмотрим пример ниже:

if(user.getPrileges().contains("admin")) {
    // Do something
}

становится

if(user.isAdmin()) {
    // Do something
}

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

deadalnix
источник
1
Кстати, пример иллюстрирует закон Деметры.
MaR
1

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

l0b0
источник
0

Если вы видите, что он дублирован, вы должны найти способ централизовать его.

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

Что делать, если вам нужно проверить что-то еще?

Собираетесь ли вы найти все места, где вам нужно добавить дополнительную проверку или просто изменить функцию?

Танос Папатанасиу
источник
... с другой стороны, что, если есть очень высокая вероятность того, что вы никогда не добавите что-то к этому. Ваше предложение по-прежнему остается лучшим вариантом действий в этом случае?
EpsilonVector
1
@EpsilonVector мое эмпирическое правило заключается в том, что если мне придется изменить его один раз, то я мог бы также реорганизовать его. Итак, в вашем случае я бы оставил это, и если бы мне пришлось изменить его, это стало бы функцией.
Танос Папатанасиу
0

Почти всегда хорошо, если выполняются следующие условия:

  • улучшает читаемость
  • используется в ограниченном объеме

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

MaR
источник