Initial work on moving the Processor for messages to a separate class #21
Conversation
| if (is_null($processor)) { | ||
| $this->processor = new \Productsup\Flexilog\Processor\DefaultProcessor(); | ||
| } else { | ||
| $this->processor = $processor; |
There was a problem hiding this comment.
$this->processor = $processor ?? new \Productsup\Flexilog\Processor\DefaultProcessor;There was a problem hiding this comment.
That would make the entire codebase PHP7 minimum, since this feature didn't exist before. http://php.net/manual/en/migration70.new-features.php#migration70.new-features.null-coalesce-op
Currently the codebase is PHP5.6 compatible.
There was a problem hiding this comment.
What about:
if (is_null($processor)) {
$processor = new \Productsup\Flexilog\Processor\DefaultProcessor();
}
$this->processor = $processor;
| @@ -30,9 +33,20 @@ | |||
| * | |||
| * @param string $minimalLevel the minimal severity of the LogLevel to start logging with | |||
| * @param integer $verbose the Verbosity of the Log | |||
| * @param array $additionalParameters optional additional parameters required for the Handler | |||
| * @param \Productsup\Flexilog\Processor $processor the Processor to be used for the Handler, if none supplied | |||
There was a problem hiding this comment.
\Productsup\Flexilog\Processor can be replaced with ProcessorInterface
| public function __construct($minimalLevel = 'debug', | ||
| $verbose = 0, | ||
| array $additionalParameters = array(), | ||
| ProcessorInterface $processor = null) |
There was a problem hiding this comment.
PSR-2 Compliant would be:
public function __construct(
$minimalLevel = 'debug',
$verbose = 0,
array $additionalParameters = array(),
ProcessorInterface $processor = null
) {
| if (is_null($processor)) { | ||
| $this->processor = new \Productsup\Flexilog\Processor\DefaultProcessor(); | ||
| } else { | ||
| $this->processor = $processor; |
There was a problem hiding this comment.
What about:
if (is_null($processor)) {
$processor = new \Productsup\Flexilog\Processor\DefaultProcessor();
}
$this->processor = $processor;
| /** | ||
| * Abstract Handler to simplify the implementation of a Handler Interface | ||
| */ | ||
| abstract class AbstractHandler implements HandlerInterface | ||
| { | ||
| protected $logger = null; | ||
| protected $processor = null; |
There was a problem hiding this comment.
Add docblock:
/**
* @var ProcessorInterface
**/
| * @return HandlerInterface $this | ||
| */ | ||
| public function setLogger(\Productsup\Flexilog\Logger $logger) | ||
| public function setProcessor(\Productsup\Flexilog\Processor $processor) |
There was a problem hiding this comment.
Add docblock.
It the property $processor now a Processor or a ProcessorInterface? Big difference imo.
| */ | ||
| abstract class AbstractProcessor implements ProcessorInterface | ||
| { | ||
| protected $handler = null; |
There was a problem hiding this comment.
Add:
/**
* @var \Productsup\Flexilog\Handler
**/
| * | ||
| * @param string $level | ||
| * @param string $message | ||
| * @param array $context |
| * | ||
| * @param string $level | ||
| * @param string $contextKey | ||
| * @param string $contextObject |
| } | ||
|
|
||
| /** | ||
| * Return a separator between the Message and the Context if applicable. |
| { | ||
| // build a replacement array with braces around the context keys | ||
| $replace = array(); | ||
| foreach ($context as $key => $val) { |
There was a problem hiding this comment.
Maybe context preparation should take place automatically inside this method? Or indicate that $context should actually be a preparedContext, since that handles most data types.
Currently in Flexilog the AbstractHandler took care of a lot of things, and some Handlers also were not generic enough to be easily put to use for specific tasks.
The introduction of a Processor should allow this to be easily extendable depending on the task.
As an example I've included a cleaned up FileHandler and a new LogfileHandler.