tags:

views:

101

answers:

6

Here is a scenario

class page {
     public $name;
     public $title;

     public function showhead() {
           return "<head><title>".$this->title."</title></head>";
     }
}
$mypage = new page;
$mypage->title = "My Page title";
$mypage->showhead();

and another scenario

class page {
     public $name;
     public function showhead($title) {
           return "<head><title>".$title."</title></head>";
     }
}
$mypage = new page;
$mypage->showhead("My Page title");

Among these methods, which is better and which should be avoided? And why?

+1  A: 

Without any additional information, I would say that the first one is "better," because it seems to me that $name and $title are properties belonging to page, therefore, it should be a class member. However, if you feel that the second version is more applicable to your situation, then by all means use it.

In silico
+7  A: 

I guess it depends on whether you need that title ever again. if you do then make property for storage and retrieval. if you only need it that one time then use the method parameter.

spinon
+5  A: 

There is always some tension between passing parameters around (either individually or in immutable aggregate types, which don't really exist in PHP) and storing them somewhere (be it class properties, globals, whatever). One of the benefits of OOP is that you can store state in objects and benefit from encapsulation (prevents many accidental overwriting of data) while simultaneously avoiding polluting the symbol tables with variables. Using these mutable objects has its own sets of problems, especially if we go into multi-threaded programming, but that's a smaller concern in PHP.

In your particular case, I think it would be a better idea to store the title in the object. As In silico said, it seems to belong to the page, and on top of that you can do stuff like:

$page = new page;
$page->setTitle("whatever");
...
function doStuff($page) {
    ...
    $page->showhead();
}

And then you don't have to pass $page together with the title.

Artefacto
+3  A: 

IMO: because the ->showhead() has less semantic relationship to the $title than the object itself, you should assing $title either as property, or better via the object constructor:

class page {
    function __construct($title="") {
        $this->title = $title
    }

So you can either new page("My page") or assign it later $page->title=..., depending on when you have it available.

mario
And more importantly; You can now declare the property protected and thereby make the object immutable.
troelskn
+1  A: 

On a side note for best practices, it would be a good idea to not access the object member directly, but instead make it protected and set it/get it through public methods instead, something like this

class foo
{
    protected $title;

    public function setTitle($title)
    {
        $this->title = $title;
    }

    public function getTitle()
    {
        return $this->title;
    }
}

This removes the need for any other code working with this data needing to knowing how it is actually stored and instead provides an interface to it instead, thus encapsulating(isolating) the data. http://en.wikipedia.org/wiki/Information_hiding

tsgrasser
+3  A: 

You should ask yourself: does title belong to a page? Or is it something that a page uses.
These are the main heuristics I use to find out how an entity needs to be designed in OOP:

  • Compisition is a relationship of has-a. Does a page have a title? The answer to this question is yes.
  • Inheritance is a relationship of a is-a. Is the page a title? The answer to this question is no. Note that you should prefer composition over inheritance whenever possible. Usually you can use either of them.
  • Passing a parameter to a function is a relationship of uses-a. Does a page use a title? The answer to this question is yes. However has-a is a stronger relationship then uses-a. Again sometimes you can use both but you should prefer has-a to uses-a when possible.
the_drow