Issues

ZF-8953: All public methods of Zend_Log should return the current instance

Description

All public methods of Zend_Log should return the current instance (return $this) so a fluent usage is possible.

Sometimes its necessary to add more writers or event items at once and a fluent interface makes this nicer to implement. There should be no downside because in its current state all public methods return nothing.

$logger->addWriter($writer);
$logger->setEventItem('foo1', 'bar1');
$logger->setEventItem('foo2', 'bar2');

vs.

$logger->addWriter($writer)
       ->setEventItem('foo1', 'bar1')
       ->setEventItem('foo2', 'bar2');

Comments

Reassigned to @matthew.

Attached the patch.

Fixed with the r22568.

Doesn't this - strictly spoken - break bc, and therefore may not be committed into the release branch?

Soon I will revert branch but I believe should be implemented in the minor release ;).

Reverted in branch r22632.

I was originally going to re-open the issue, but I will note that it does introduce a slight BC break.

Technically speaking, we usually characterize BC breaks as one of the following:

  • Changing the signature (e.g., adding new required arguments; changing the return value type; making argument typehints more restrictive)
  • Making visibility more restrictive (e.g., moving from public to protected or private; moving from protected to private)

However, changing the visibility at all actually can introduce some BC breakage. If a method is marked as private, and we change it to protected, we can cause BC breakage in the case where somebody has extended the class and overridden that method, while retaining the private visibility. In this case, they will get an E_FATAL as their method is now more restrictive than the method they're extending.

That said, they also need to override a public or protected method in the first place in order to even call the formerly private method. I would hope they have a good test suite in place. :)

I still think this is a good idea, but it's easier to call this sort of thing out in a minor release than in a maintenance release.

Thanks you Matthew and Freeaqingme by reported. Only applied to branch because I saw a similar issue ZF-9913 be applied in release 1.10.6.