Search code examples
phploggingdesign-patternssolid-principlessingle-responsibility-principle

Add global logs in app - what about SOLID


I have php app, it isn't compilant with SOLID principles, but whole team tries refactore code on changes. I must add global logs (stored in one of databases), saved on creations on updates on models. That models don't use ORM. First solution: create static logger and call after operation on model:

public function save(ObjectEntity $entity)
{
    // Some code to prepare entity
    $this->insert($entity);

    Logger::saveLog('Object has been saved');

    // Or maybe better - separate classes for logs with interface 
    Logger::save(new LogObjectEntitySave());
}

But... is it correct? I think is also not compilant with SOLID and would like not make new mess in current. Where should I add something like this - on models, or maybe on controllers after calling model saving:

public function saveAction()
{
    // Controller code here

    $model->save($objectEntity);
    Logger::save(new LogObjectEntitySave());
}

But there is question: what about one action to save and also update data in model (I add new element when I don't have existing id)? If/else and two log classes.. still looks bad. No idea how should it be right.


Solution

  • 1) perform the logging actions in the Model save() not in the controllers, in the saveAction. Else you are going to have to find EVERY $model->save($objectEntity) piece of code and add the logging. If you forget one of them, your logging feature is not reliable, your logs then lie to you, you cannot trust them, they become useless.

    2) If you think that you violate the S in SOLID because your Model save() action does 2 things (the insert() and the saveLog), no it does not. Because it delegates the responsibility to perform the logging actions to saveLog(). Calling saveLog() is fine and does not violate SOLID.

    3) the static Logger class is indeed not the best choice (cannot be easily replaced by another implementation, hardcoded everywhere ...) but if you application does not have Dependency Injection abilities such as a container, this is not a bad choice: it is easy to use, easy to control and maintain. If it makes your life as a developer easier, that is already a good step forward :) .

    If you have Dependency Injection, inject a Logger service just like the Symfony framework does for example.

    4) For your last question about when you have a save AND an update, I think you will need the if/else logging case. Yes, this makes it complex (probably good to wrap it in a private function to hide its complexity) but your logs will be clear and accurate. And this is important. Your logging action will be complex because what is being logged is complex. There is nothing you can do about it.

    Hope this helps