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->mProductUnitsas$unit){$ret.=hHtml::Clear();$ret.=$this->RenderInfoRow($unit->showUnit(),$unit->RenderPriceWithCurrency(),$unit->AvailabilityMessage());}...privatefunctionRenderInfoRow($name,$price,$availbility){return'<div>'.$name.'</div>'.'<div>'.$price.'</div>'.'<div>'.$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->mProductUnitsas$unit){$ret.=$this->RenderInfoRow($unit);}privatefunctionRenderInfoRow(oProductUnit$u){returnhHtml::Clear().'<div>'.$u->showUnit().'</div>'.'<div>'.$u->RenderPriceWithCurrency().'</div>'.'<div>'.$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.