Do One Thing - Как далеко, чтобы принять это правило? - PullRequest
7 голосов
/ 27 августа 2009

Итак, в книге «Чистый код» есть правило «Делай одно». Но как далеко мы действительно должны это сделать.

Например, следующие утверждения:

Settings.Default.BaudRate = baudRate;
Settings.Default.COMPort = port;
Settings.Default.DataBits = dataBits;
Settings.Default.Handshake = handshake;
Settings.Default.Parity = parity;
Settings.Default.ReadTimeout = readTimeout;
Settings.Default.WriteTimeout = writeTimeout;
Settings.Default.CommunicationTimeout = communicationTimeout;
Settings.Default.Save(); 

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

У меня есть это в одной функции. Вы действительно взяли бы это приложение? и есть один метод для каждого параметра?

Когда вы придерживаетесь этого правила, а когда нет?

Ответы [ 11 ]

19 голосов
/ 27 августа 2009

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

18 голосов
/ 27 августа 2009

Выглядит совершенно справедливо для меня. Очевидное имя метода для этого кода - SaveSettings, что указывает на то, что метод делает только одно. Не о чем беспокоиться.

6 голосов
/ 27 августа 2009

Да, каждый метод должен делать только одну вещь. Но что это за вещь?

Это зависит от уровня абстракции, в котором находится ваш метод . Метод (свойство) для сохранения одного параметра является довольно низкой абстракцией. Следующей более высокой абстракцией будет предложенный метод SaveSettings.

В самом верху у вас есть единственный метод / функция main, который также выполняет только одно: вся программа ...

6 голосов
/ 27 августа 2009

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

4 голосов
/ 27 августа 2009

Я не читал эту конкретную книгу, но что касается концепции ...

«Одна вещь» не означает «одна строка кода». «Одна вещь» означает, что все в функции должно быть логически связано.

Я бы поспорил с некоторыми из предыдущих постеров, сказав, что «saveSettings» - это «одна вещь». Может быть, это просто небрежность с формулировками, но я воспользуюсь возможностью, чтобы указать на потенциальную ловушку. В вашем случае это больше похоже на «saveCommunicationSettings», которое, я думаю, легко соответствует определению «одна вещь». Если бы вы добавили «Settings.Default.customerLoyaltyDiscount = ...» в этот список, я бы сказал, что вы, вероятно, находитесь на опасной почве, потому что вы сейчас смешиваете настройки связи с настройками расчета цены.

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

4 голосов
/ 27 августа 2009

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

Я бы следовал Принципу единой ответственности , который обобщается как:

Никогда не должно быть более одной причины для изменения класса

и может в равной степени применяться к методам. В вашем примере когда-нибудь будет причина обновлять настройки, но не сохранять их?

4 голосов
/ 27 августа 2009

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

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

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

2 голосов
/ 27 августа 2009

В нашей команде мы стараемся следовать одной цели для каждой функции. Чтобы помочь разработчикам, мы добавили предложение в наши стандарты, чтобы они рассмотрели рефакторинг, если функция превышает 25 строк. Поэтому, если у вас есть 100 строк свойств настройки кода, вы можете рассмотреть возможность их разделения по категориям, таким как SaveUserSettings, SaveNetworkSettings и т. Д.

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

1 голос
/ 27 августа 2009

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

0 голосов
/ 19 ноября 2015

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

public function buildPage() {
   $page = header();
   $page .= body();
   $page .= footer();
   return $page;
}

эта функция делает только одно, а все подфункции являются частью фразы buildPage

public function sendMail() {
   $user = $this->getUser();
   $this->mailer->content("send this message!")
       ->to($user->email)->send();
}

для этой функции требуется электронная почта пользователя, поэтому с помощью функции getUser sendMail удается получить электронную почту пользователя. но $ user = getUser () не описаны в фразе sendMail Лучшая практика для этой функции

public function sendMail($email,$message) {
   $this->mailer->content($message)->to($email)->send();
}
...