Какой вариант кода чище и правильнее?
Что лучше, 3 раза просто вызвать эту функцию и все, либо пустить её через цикл и чтобы она там 3 раза вызвалась и во время цикла, через условия заменять значения на каждой итерации, я понимаю, делаю одно и тоже, только еще условия добавляя, но все же, что более чище и читабельнее и грамотнее??
1 вариант или 2?
public function setAgrClause($data)
{
//1 вариант
$classIsn = 198390;
$clauseIsn = 11110;
for ($i = 0; $i < 3; $i++) {
if ($i > 0) {
$classIsn = 5377;
$clauseIsn = 2999;
}
if ($i > 1) {
$classIsn = 471491;
$clauseIsn = 471481;
}
$this->kias->setAgrClause($this->userIsn, $data['agrISN'], $classIsn, $clauseIsn);
}
//2 вариант
$this->kias->setAgrClause($this->userIsn, $data['agrISN'], 198390, 11110);
$this->kias->setAgrClause($this->userIsn, $data['agrISN'], 5377, 2999);
$this->kias->setAgrClause($this->userIsn, $data['agrISN'], 471491, 471481);
return 1;
}
Ответы (1 шт):
Первый вариант это вообще что-то с чем-то. Явная попытка натянуть сову на глобус. Зачем так запутывать код - вообще не понятно.
Если вас напрягает то что у вас идут подряд три вызова одного и того же метода, но с разными параметрами и хочется сделать цикл и красиво, то логичнее использовать массив с данными и уже на его основе делать цикл.
Как-то так, например,
$params = [
['classIsn' =>198390, 'clauseIsn' => 11110],
['classIsn' =>5377, 'clauseIsn' => 2999],
['classIsn' =>471491, 'clauseIsn' => 471481],
];
foreach($params as $isn) {
$this->kias->setAgrClause(
$this->userIsn,
$data['agrISN'],
$isn['classIsn'],
$isn['clauseIsn']
);
}
Следующим шагом по рефакторингу, может быть вариант, когда этот массив определяется не в методе, а передается в качестве параметра. Т.е. сам по себе метод становится независимым от всякого рода магических констант. Ну и потом для расширения системы есть место куда расти. Может быть потом сделаете чтобы эти параметры брались из конфигруационного файла или из базы данных. И таим образом изменение чисел не приведет к правкам кода.