Page 1 of 1
Property distribution and method independence
Posted: Wed Dec 13, 2006 7:15 pm
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.
Posted: Wed Dec 13, 2006 9:39 pm
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.
Posted: Wed Dec 13, 2006 10:42 pm
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.
Posted: Thu Dec 14, 2006 12:28 am
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 */ }
}
Posted: Thu Dec 14, 2006 5:39 am
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;
}
Posted: Mon Dec 18, 2006 10:01 am
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

Posted: Mon Dec 18, 2006 1:19 pm
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.