Property distribution and method independence

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

Post Reply
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Property distribution and method independence

Post by Ollie Saunders »

Which is better and why? I've failed to reach a decision.

Code: Select all

class Foo
{
    private $_prop;
    public function __construct($prop)
    {
        $this->_prop = $prop;
    }
    public function doSomething()
    {
        $prop = $this->_prop;
        $this->_a($prop);
        $this->_b($prop);
    }
    private function _a($prop) { /* does something with $prop */ }
    private function _b($prop) { /* does something with $prop */ }
}
or

Code: Select all

class Bar
{
    private $_prop;
    public function __construct($prop)
    {
        $this->_prop = $prop;
    }
    public function doSomething()
    {
        $this->_a();
        $this->_b();
    }
    private function _a() { /* does something with $this->_prop */ }
    private function _b() { /* does something with $this->_prop */ }
}
Methods _a() and _b() in Foo are independent of the object which can only be a bonus.
But methods _a() and _b() in Bar can be called without a parameter resulting in less code duplication.

Edit: Actually I have made a decision. I'm going with Bar, less duplication = good, separate from object = don't need.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

I agree that Bar is probably better, but I am not sure for the reasons you give. I think there are separate reasons to use parameters/locals and properties/globals -- though there is some overlap in those reasons.
(#10850)
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

The distinction here, I think, is that _a() and _b() don't edit $prop, they simply take it and do something with it. But if you follow this to the logical extreme and parameterize $prop, you end up with what are essentially static functions. I'd stick with Bar, and only refactor into a parameter if it's direly necessary for some unit testing gymnastics.
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

The example you gave doesn't say much about what is going on in _a and _b.. If that suggests that they don't call other methods that need to know about the 'shared context (=$prop)' i'd probably end up with a FooBar

I'd probably end up with FooBar...

Code: Select all

class FooBar {
 public function doSomething($prop) {
  FooBar::_a($prop);
  FooBar::_b($prop);
 }

 private function _a($prop) { /* does something with $prop */ }
 private function _b($prop) { /* does something with $prop */ }
}
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

But if you follow this to the logical extreme and parameterize $prop, you end up with what are essentially static functions
Yes that was what I was getting at about being independent from the object.
I'd stick with Bar, and only refactor into a parameter if it's direly necessary for some unit testing gymnastics.
Because of the less duplication thing right?
I think there are separate reasons to use parameters/locals and properties/globals
But here they seem to do the same. What would you say those reasons are?
The example you gave doesn't say much about what is going on in _a and _b
This is the code in question. Out of context its pretty confusing to look at but as you can see _headerWithoutNotifications() makes lots of _get*($ent) calls

Code: Select all

private function _headerWithoutNotifications()
    {
        $ent = $this->_entity;                              // look: the origin of $ent
        $attr = array('id' => $ent->id);
        if ($ent->isMaster) {
            if ($ent->visible) {
                return $this->_tagPart('form', 1, $this->_getFormAttr($ent))   // lots
                     . $this->_tagPart('fieldset', 1, $this->_getFieldSetAttr($ent))    // of
                     . $this->_getLegend($ent->attributes->label, $ent->accessKey);   // calls
            }
            return $this->_tagPart('form', 1, $this->_getFormAttr($ent)); // and here
        }
        if ($ent->visible) {
            return $this->_tagPart('fieldset', 1, $this->_getFieldSetAttr($ent)) // and here
                 . $this->_getLegend($ent->attributes->label, $ent->accessKey);
        }
        return $this->_tagPart('div', 1, $this->_getDivAttr($ent));
    }



    private function _getFormAttr($ent)             // by using this parameter I have avoided...
    {
        // this line:
        // $ent = $this->_entity;
        $out = array('id'             => $ent->id,
                     'action'         => $ent->action,
                     'method'         => $ent->method,
                     'accept-charset' => OF::$enc->getName());
        if ($ent->visible) {
            return $out + $ent->attributes->js->get() + $ent->attributes->custom;
        }
        $out['title'] =  $ent->attributes->tooltip;
        return $out;
    }



    private function _getDivAttr($ent)
    {
        // same here
        return array('id'       => $ent->id,
                     'title'    => $ent->attributes->tooltip);
    }



    private function _getFieldSetAttr($ent)
    {
        // and here
        $out = array('class'    => $ent->attributes->class->get(),
                     'style'    => $ent->attributes->css->get(),
                     'title'    => $ent->attributes->tooltip);
        if ($ent->isMaster) {
            return $out;
        }
        return $out + array('id' => $ent->id) + $ent->attributes->js->get() + $ent->attributes->custom;
    }
User avatar
raghavan20
DevNet Resident
Posts: 1451
Joined: Sat Jun 11, 2005 6:57 am
Location: London, UK
Contact:

Post by raghavan20 »

I have not read all the posts but I would like to make a general comment.

Your second method is preferred because you can make it as even public and make other classes/functions use it if operation is allowed.
The first method is preferred normally when you think there is no other object in the world want to use/invoke this method and this operation is used only in its own object world.


EDIT:

I would like to point you one small thing in your code which you can avoid.

Code: Select all

class Foo
{
    private $_prop;
    public function __construct($prop)
    {
        $this->_prop = $prop;
    }
    public function doSomething()
    {
        $prop = $this->_prop; ###THIS LINE IS COMPLETELY UNNECESSARY AS YOU CAN USE $this->_prop IN THE FOLLOWING FUNCTION PARAMETERS; REMEMBER IN PHP OBJECTS ARE SENT BY REFERENCE BY DEFAULT, YOU HAVE TO CLONE TO GET A NEW COPY####
        $this->_a($prop);
        $this->_b($prop);
    }
    private function _a($prop) { /* does something with $prop */ }
    private function _b($prop) { /* does something with $prop */ }
}
Thanks :)
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Your second method is preferred because you can make it as even public and make other classes/functions use it if operation is allowed.
In this case I do not want to do that. Also why couldn't I do that with the first method? It would mean something different sure but there nothing there saying it can't be public.
The first method is preferred normally when you think there is no other object in the world want to use/invoke this method and this operation is used only in its own object world.
Yes. You have just described what I was thinking but couldn't put into words and yes that is precisely why I agree that Bar is superior to Foo.
THIS LINE IS COMPLETELY UNNECESSARY AS YOU CAN USE $this->_prop IN THE FOLLOWING FUNCTION PARAMETERS;
That's a throwback from the full version where I use $prop instead of $this->_prop everywhere to improve performance and save my fingers.
REMEMBER IN PHP OBJECTS ARE SENT BY REFERENCE BY DEFAULT, YOU HAVE TO CLONE TO GET A NEW COPY
For PHP 5, yes.
Post Reply