search

LogentriesCommunity / le_php

Logentries support for PHP
29 stars 17 forks source link

Don't rely on global variables #11

Open evert opened 9 years ago

evert commented 9 years ago

Relying on global variables is considered a bad practice in PHP. It would be best not to promote or suggest using this.

You have a fairly decent API in LeLogger.php, but promoting the API in lelogger.php is something I would suggest you avoid.

Rather than making it 'the API' it may be better to show it off as an example that can be re-used, but shouldn't be directly included. I imagine lelogger.php was added to ease setting up the LeLogger object, but it may be better to do this in the form of a factory method that takes an array for setup.

Just a thought.

evert commented 9 years ago

Your setup instructions for the PHP package is also super odd. It promotes usage of the object as a singleton, and the actual example will fail if it's used in more than 1 place in a PHP application.

The require_once statement will work the first time, and simply fail the second time.

StephenHynes7 commented 9 years ago

Hey @evert thanks for the feedback! Yes I agree logentries.php has become quite bloated in terms of overall functionality. I would imagine the original idea is that it acts almost like a configuration file for a User to manipulate.

May be a good idea instead to have it as an example as you suggest and instead let the User decide how they want to setup.

developer99 commented 9 years ago

Doesn't work on PHP setup on a Windows box either as the global variables for the severity levels do not automatically exist as I can see. I had to add them for an app. Needs to be fixed.

cbschuld commented 9 years ago

Hi @evert @StephenHynes7 - I had a similar issue with the "api" feel, the reliance on globals I also wanted a smooth integration with composer I decided to retool it completely using PSR3. I created a new class at cbschuld/LogEntries (https://github.com/cbschuld/logentries) and stuffed her into composer:

composer require cbschuld/logentries:1.*

FYI Only; extremely open to feedback.

wujekbogdan commented 6 years ago

There's also another solution. Logentries handler for Monolog logger: https://github.com/LogentriesCommunity/logentries-monolog-handler