patternphpMinor
Setting properties inside of a class
Viewed 0 times
propertiessettingclassinside
Problem
Instead of using multiple setter functions within a class like
etc
I've been using a method from a base class that all other classes have access to.
So I can just use it like so
Is this a bad practice that I'm getting into or is this an okay way of setting properties?
public function setAction()
public function setMethod()etc
I've been using a method from a base class that all other classes have access to.
public function set_properties($properties) {
foreach ($properties as $key => $value) {
$this->$key = $value;
}
}So I can just use it like so
$Class->set_properties(array('action' => 'page.php', 'method' => 'get'));Is this a bad practice that I'm getting into or is this an okay way of setting properties?
Solution
Having individual setters has a couple of advantages:
A dedicated setter allows you to put dedicated validations for these properties. You could obviously put them in the bulk setter as well, but that will make the bulk setter large and unwieldy very quickly. You want methods to do one dedicated thing instead. That keeps them maintainable.
Dedicated setters clearly communicate which properties can be set. With a bulk setter, it needs documentation or a look at the source code to learn what I can legally set. I usually don't want to look at your class code. I just want to look at the public API and understand how I can use that class.
That's not to say bulk setters are bad per se. If you know folks are likely to set two or more values after another because they naturally go together, you could allow for a
On a side note: Your bulk setter will create properties on the fly for those keys that are not properties in the class. While you could consider that a feature, it makes debugging harder because a developer has to know the object's current state at runtime to know what properties it has.
On another side note: a base class is usually a code smell. It tends to amass unrelated functionality and turn in a God object quickly. It also hampers reuse because all your classes will depend on it then.
A dedicated setter allows you to put dedicated validations for these properties. You could obviously put them in the bulk setter as well, but that will make the bulk setter large and unwieldy very quickly. You want methods to do one dedicated thing instead. That keeps them maintainable.
Dedicated setters clearly communicate which properties can be set. With a bulk setter, it needs documentation or a look at the source code to learn what I can legally set. I usually don't want to look at your class code. I just want to look at the public API and understand how I can use that class.
That's not to say bulk setters are bad per se. If you know folks are likely to set two or more values after another because they naturally go together, you could allow for a
setFooAndBar($foo, $bar) method. But as you can see, that's still much more explicit than a generic set(array $data). Also one could argue that if you need to set arguments that naturally go together, you rather want them to be in an object of their own and pass that object then.On a side note: Your bulk setter will create properties on the fly for those keys that are not properties in the class. While you could consider that a feature, it makes debugging harder because a developer has to know the object's current state at runtime to know what properties it has.
On another side note: a base class is usually a code smell. It tends to amass unrelated functionality and turn in a God object quickly. It also hampers reuse because all your classes will depend on it then.
Context
StackExchange Code Review Q#29617, answer score: 2
Revisions (0)
No revisions yet.