Слишком большие объекты данных

Question

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

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

Например:

• номера телефонов
• выбранных кампаний
• выбранные шаблоны
• выбранные контакты
• выбранные группы
• и около 2-4 типов информации подробнее

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

Чтобы повысить удобочитаемость, я ввел класс Data с именем CostCountingData со читаемыми геттерами и сеттерами для всех этих свойств.

Но я не думаю, что этот код стал намного читабельнее после этого рефакторинга:

$cost_data = new CostCountingData();
$cost_data->setNumbers($numbers);
$cost_data->setContacts($contacts);
$cost_data->setGroups($groups);
$cost_data->setCampaigns($campaigns);
$cost_data->setUser($user);
$cost_data->setText($text);
$cost_data->setTotalQuantity($total_quantity);

$CostCounter = new TemplateReccurentSendingCostCounter($cost_data);  
$total_cost = $CostCounter->count();

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

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

Что ты об этом думаешь?

Answer 1

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

Для этого вы должны изменить свои методы на CostCountingData, чтобы они все возвращали исходный объект. Таким образом, вы можете переписать свой верхний фрагмент кода следующим образом:

$cost_data = new CostCountingData();
$cost_data
 ->setNumbers($numbers)
 ->setContacts($contacts)
 ->setGroups($groups)
 ->setCampaigns($campaigns)
 ->setUser($user)
 ->setText($text)
 ->setTotalQuantity($total_quantity);

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

Answer 2

Здесь есть несколько возможностей:

• Если вам нужен только несколько параметров каждый раз, подход с именованными параметрами , предложенный в другом месте этого ответа, является хорошим решением.

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

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

Сделайте шаг назад:

• Являются ли восемь отдельных аргументов действительно не связанными, или вы можете составить промежуточные объекты, которые немного лучше отражают реальность ситуации? Товары? пункты счета? Что-то еще?

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

Answer 3

Если бы мне понадобился объект данных (а сами классы, называемые «данными», сами по себе пахнут), я бы установил его значения в его конструкторе. Но интересный вопрос: откуда эти ценности? Источник значений сам по себе должен быть каким-то объектом, и, вероятно, он действительно вам интересен.

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

class CostStuff {

   constructor CostStuff( $postdata ) {
       $mypost = $postdata;
   }

   function User() {
      return $mypost[ "user_name" ];
   }

   ...

}

Answer 4

Что вы думаете не так с вашим фрагментом кода? Если у вас есть несколько переменных, которые вы уже использовали, и теперь вам нужно добавить их в $ cost_data, вам нужно будет установить каждую из них по очереди. Обойти это невозможно.

Однако вопрос в том, почему у вас есть эти переменные $ numbers, $ contract и т. Д., Которые вы сейчас решаете, что вам нужно перейти в $ cost_data?

Возможно, вы ищете рефакторинг, что точка в коде, где вы устанавливаете $ numbers, должна фактически быть точкой, в которой вы устанавливаете $ cost_data-> numbers, а наличие переменной $ numbers вообще излишне?

1007 * Е.Г. *

$numbers=getNumbersFromSomewhere() ; 
// do stuff
$contracts=getContracstFromSomewhere() ;
// do stuff 
$cost_data=new dataObject() ; 
$cost_data->setNumbers($numbers);
$cost_data->setContracts($contracts) ; 
$cost_data->someOperation() ; 

становится

$cost_data=new dataObject() ; 

$cost_data->setNumbers(getNumbersFromSomewhere()) ; 
// do stuff
$cost_data->setContracts(getContractsFromSomewhere()) ; 
// do stuff
$cost_data->someOperation() ;