I recently came across a situation where a few of my setProperty() methods where exactly the same. Since I’ve gotten into improving my coding, this bugged me because it was repetitious code.
Example:
public function setFoo($foo=null) {
if (is_string($foo)||is_numeric($foo)) {
$this->foo=$foo;
return $this;
}
throw new Custom_Example_Exception("Please provide a proper string or numeric",Custom_Example_Exception::INVALID_TYPE);
}
public function setBar($bar=null) {
if (is_string($bar)||is_numeric($bar)) {
$this->bar=$bar;
return $this;
}
throw new Custom_Example_Exception("Please provide a proper string or numeric",Custom_Example_Exception::INVALID_TYPE);
}
Notice a pattern there? Me too. If all my setters were like this, I could do
public function setProperty($property=null) {
if (is_string($property)||is_numeric($property)) {
$this->{$property}=$property;
return $this;
}
throw new Custom_Example_Exception("Please provide a proper string or numeric",Custom_Example_Exception::INVALID_TYPE);
}
with appropriate traps in the __call() and maybe even __set() methods.
However, my situation is that not all the setters for my class follow this pattern so that solution means that I have to make __call() look for specific setters.
public function __call($method,$args) {
$trapThese=array("Foo","Bar","Baz");
preg_match('/set(*.?)/im',$method,$matches);
if (!empty($matches)&&in_array($matches[1],$trapThese)) {
if(is_string($args[0])||is_numeric($args[0])) {
$propertyName=strtolower($matches[1]);
$this->{$propertyName}=$args[0];
}
throw new Custom_Example_Exception("Please provide a proper string or numeric.\nException trapped in call to $method",Custom_Example_Exception::INVALID_TYPE);
} else {
$this->{$method}($args);
}
}
And I’m pretty sure that this code snippet ($this->{$method}($args);) is the Wrong Way of Doing It. So I’ll just have to settle for the redundant setFoo(),setBar,etc even though a part of me knows that there has to be a Better Way. Or maybe there is just No Way of Doing It Well.
I think it is probably only a matter of preference but in terms of testability, elegance, maintainability, whatever…what do you prefer, doing a magic __call() or making setFoo(),setBar(),setBaz,etc.
The $this->{$method}($args); is wrong because if execution has made it into __call, the class doesn’t have a method name $method. So re-issuing the call will just get you back into __call() and eventually your call stack will reach its limit. So the way to set up what you’re proposing is to write all the unique setters and then have the __call method handle any that don’t have a unique one written.
Maybe there are pressing reasons to use explicit methods, but what might be related and suites my needs very well, is in stead of using a magic __call() catcher I use __get() and __set() functions to access my object properties.
In a bit more detail, I make all my properties protected (or private if they should not be accessable to sub classes). Since protected and private properties are invisible from the outside of the object, I can use a __get() method to capture any attempt to access a property I would like to make available as public. Similarly I use a __set() to give write access to properties of my choosing.
The only real drawback I encounter with this is that from the code I can’t directly see if a property is public or not. However, this is easily ‘fixed’ with a proper comment block and can be done in such a way that I can get a distinction in IDE’s like netbeans and eclipse through autocompletion.
As an extra bonus this way of working solves a problem that I heard somebody complain about some while ago. Don’t remember who it was or where, but the complaint was that in PHP your don’t have such a thing as a read-only public property. By providing read access to a property through a __get(), but omitting the write access through a __set(), you effectively get a read-only public property. I found this “feature” by accident (actually through a typo bug) but have been very happy with it and have been using it ever since. I would be surprised though if I was the only one using these methods this way.
Anyway, I hope this information was usefull in one way or another.
@pgraham thx. I knew it was wrong but couldn’t put my finger onto why.
I think that anything that can be done which over complicates an original piece of code should be scrapped. Simplicity rules out complexity any day of the week even if it cuts down on runtime. Just because a feature exists doesn’t always mean it should be done. What happens to your code later on when you need to add some new features. I’m sure you could make a check function which does the same thing you are doing over and over again.
private function checkVar($name,$value){
if(is_string($value)||is_numeric($value)){
$this->${$name} = $value;
return $this;
}else return false;
}