DEV Community


A small refactoring example

PHP and cigars
・2 min read

I'm currently reviewing a small piece of code of a new co-worker. As he comes back to work next week I will share a small example how to refactor the code here, so I won't forget to mention it to him.

I found this:


foreach ($p->mProductUnits as $unit) {
    $ret .= hHtml::Clear();
    $ret .= $this->RenderInfoRow(


private function RenderInfoRow($name, $price, $availbility){

        '<div class="col-l-4 col-m-4">'.$name.'</div>'
        .'<div class="col-l-2 col-m-2">'.$price.'</div>'
        .'<div class="col-l-4 col-m-4">'. $availbility.'</div>';


What problems do we have here? Hungarian notation? Well, yes, but that is my fault and is due to the framework i wrote. Besides that?

Wright. The single responsability principle is hurt. Also Uncle Bob teached us, that one parameter to a function is ok. In this example it is absolutely possible to use only one parameter to the function RenderInfoRow()
So lets see how this gets refactored in a first step:

foreach ($p->mProductUnits as $unit) {
    $ret .= $this->RenderInfoRow($unit);

private function RenderInfoRow(oProductUnit $u) {

    . '<div class="col-l-4 col-m-4">' . $u->showUnit() . '</div>'
    . '<div class="col-l-2 col-m-2">' . $u->RenderPriceWithCurrency() . '</div>'
    . '<div class="col-l-4 col-m-4">' . $u->AvailabilityMessage() . '</div>'


This has more than one advantages. First: It's easier to read and less to write. But second (and in this case maybe more important): Now the IDE knows, that $u is of the type oProductUnit. So now you don't have to open the class to see the function-names. You get them presented as soon as you type $u in the function.

Discussion (0)

Forem Open with the Forem app