Исходный код
protected function _buildParamsUrl($additional = array()) {
$url = array();
$params = $this->_request->getParams();
$params = array_merge($params);
foreach ($params as $var => $val) {
if (in_array($var,$this->_neededParams) && is_scalar($val)) {
$url[$var] = $val;
}
}
$url = array_merge($url,$additional);
return $url;
}
Что не так в исходном коде
- Метод
_buildParamsUrl
возвращает массив параметров, а не URL, поэтому я бы назвал его просто_buildParams
. - Сейчас у метода две ответственности: очистить массив параметров от ненужных и смёржить его с переданным извне массивом дополнительных параметров. Также метод заключает в себе знание о том, откуда берётся исходный список параметров. Желательно эти знания и ответственности как-то разделить.
- Первый вызов функции
array_merge
выглядит загадочно: возможно, его забыли убрать после очередной модификации метода. - Названия переменных
$var
и$val
для меня выглядят неубедительно: нет полной ясности, что они значат. - Функция
array_filter
существует в PHP начиная с версии 4, безымянные функции с версии 5.3 — лучше пользоваться их комбинацией для фильтрации массивов, это гораздо прогрессивнее. Я всегда за использование паттерна MapReduce вместо циклов.
Вариант рефакторинга исходного кода
protected function _buildParams($dditional = array()) {
$needed_params = $this->_filterNeededParams($this->_request->getParams());
return array_merge($needed_params, $additional);
}
protected function _filterNeededParams($params) {
return array_filter($params, function ($param_value, $param_name) {
return in_array($param_name, $this->_neededParams) && is_scalar($param_value);
}, ARRAY_FILTER_USE_BOTH);
}
Можно пойти ещё дальше и поделить фильтрацию needed params и скалярных значений, но понять необходимость такого шага можно только исходя из требований предметной области.