0

If I chain some setters together and one of them does not return $this, then I will get a fatal error. But maybe that is a good thing.

$object = new object();
$object->set('name','foo')->set('number',12)->set('color'=>'brown');

class object {
  protected $name;
  protected $number;
  protected $color;

  protected $allowed_to_set = array('name','color');

  public function set($property,$value) {
    if(!in_array((string)$property,$this->allowed_to_set)) {
      return false;
    } else {
      $this->$property = $value;
      return $this;
    }
  }
}
  • 3
    Maybe it is. Maybe if the setters don't return $this then it isn't truly a fluid interface after all? :) – MetaFight Feb 22 '14 at 4:26
  • I'm just thinking about the advantages/disadvantages. When you stop program execution you know for sure there's been an error without a doubt. But if I return $this no matter whether there was an error or not, then things keep rolling. – Buttle Butkus Feb 22 '14 at 7:35
  • 2
    Maybe throwing an exception in it (that can be caught?), yet still return $this to keep it fluid? – Jon Feb 22 '14 at 22:59
  • Why not just ignore rather than returning false since the user can't do anything about this, but log it for the developer. – James Black Feb 23 '14 at 2:23
  • Compare the code you wrote with $object->name='foo';$object->number=12;$object->color='brown'; The latter is 1. shorter, 2. faster, 3. simpler. – back2dos Feb 24 '14 at 10:42
1

I'd strongly advice against doing so:

  • Methods should always return one type. Static typed languages enforce this. Returning different types puts additional burdens on the callee. Before processing the result one has to do tedious type checks.
  • Your code can trigger fatal php errors: when set('name','foo') returns false, the next call will be false->set('number',12) which will trigger an PHP error. On of course this is not what you intended, right :)
  • Ignoring the error is as bad. The caller expects a call to have a certain effect. Getting no feedback about something going wrong implies the call succeeded.

In my opinion there are two possible approaches here:

  • Remove the fluent interface, and always return a boolean.
  • Throw an exception if the caller is not allowed to set the property. For this you should provide a way to check if the caller is allowed to set the property before. I'd consider it bad practice to try & error if I can do something.

Which one you choose is up to you and your use-case. The first indicates that invalid properties is a valid use-case where you just refuse to set the property. The second indicates an invalid use-case.

Last: don't let your design & architecture be driven by fluent interfaces. They are a nice to have usability of your API and really just a shorthand some like to use.

  • 1
    Static typed languages enforce this. That's not entirely true, since you can use a tagged union to return one of many types. The difference is that a good statically typed language can force you to handle all the possible return types. – Doval Apr 25 '14 at 15:27
  • The more code I write the more I realize that it is a pain to have a method return different types. But I don't mind PHP fatal errors like false->set('number',12) because they show me where there is an unhandled condition in the code. I can use exceptions to do that without breaking the code, but in a way I prefer the code breaks completely than continues running with unplanned for conditions. – Buttle Butkus Jul 27 '14 at 21:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.