Mar 14

Наскоро се наложи да поработя по един доста стар PHP проект, по който не бях работил от години. И там видях нещо, което ведна опреличих като code smell (Всъщност видях много такива неща, но само на това ще обърна внимание).

$cart = CartManager::getCurrentCart();
$cart->setClientInfo('atype',    $user['account_type']);
$cart->setClientInfo('fname',    $user['fname']);
$cart->setClientInfo('lname',    $user['lname']);
$cart->setClientInfo('street',   $user['street']);
$cart->setClientInfo('postnum',  $user['postcode']);
$cart->setClientInfo('epost',    $user['mail']);
$cart->setClientInfo('city',     $user['city']);
$cart->setClientInfo('company',  $user['company']);
$cart->setClientInfo('orgnum',   $user['orgnum']);
$cart->setClientInfo('phone',    $user['phone']);
$cart->setClientInfo('country',  $user['country']);

Което ако идвате от  Java света може и да ви изглежда нормално, но мен ме дразни.

На първо време направих метода setClientInfo да връща $this зада може да използвам прост method chaining. И кода стана така:

CartManager::getCurrentCart()
    ->setClientInfo('atype',    $user['account_type'])
    ->setClientInfo('fname',    $user['fname'])
    ->setClientInfo('lname',    $user['lname'])
    ->setClientInfo('street',   $user['street'])
    ->setClientInfo('postnum',  $user['postcode'])
    ->setClientInfo('epost',    $user['mail'])
    ->setClientInfo('city',     $user['city'])
    ->setClientInfo('company',  $user['company'])
    ->setClientInfo('orgnum',   $user['orgnum'])
    ->setClientInfo('phone',    $user['phone'])
    ->setClientInfo('country',  $user['country']);

Малко по-добре, но пак ми изглеждаше не естествено. Затова просто добавих възможността да може да се предава масив като аргумент. И така стана доста добре:

CartManager::getCurrentCart()->setClientInfo(array(
    'atype',    => $user['account_type'],
    'fname',    => $user['fname'],
    'lname',    => $user['lname'],
    'street',   => $user['street'],
    'postnum',  => $user['postcode'],
    'epost',    => $user['mail'],
    'city',     => $user['city'],
    'company',  => $user['company'],
    'orgnum',   => $user['orgnum'],
    'phone',    => $user['phone'],
    'country',  => $user['country']
));

За нещастие в този проект, нямаше как да направя кода стане още по-чист, без да се налага пренаписване на голяма част от проекта:

CartManager::getCurrentCart()->setClient($user);

4 коментара за "Малък PHP code smell"

  1. Иван каза:

    Точно днес ти отврях блога и се чудих няма ли да пускаш скоро ;)

    Та по темата.. Първия вариант, според мен, е далеч по-добър от втория. Отвратително много мразя свързване на методи без ясна цел ;)

    Третия начин е идеален :) По малко и от двете ;)

  2. Radoslav Stankov каза:

    Втория беше просто тест идеята ми беше да не се дефинира допълнителна променлива и т.н.

    Иначе тези 1-2 седмици не ми остана много свободно време (както винаги), че и в отпуска бях без комп. Мисля към края на този месец да по размръзя няколко стари постове :)

  3. Веселин каза:

    Нищо не си видял, аз какви проекти се налага да разджурквам понякога… На това тук му викам рай :-)

  4. Radoslav Stankov каза:

    Знам, за което се радвам. Като съм гледал познати колеги с какви неща са се сблъсквали аз съм си направо щастливец :)

Какво мислите по въпроса