[GH-ISSUE #1667] v3: Consider merging Level and LevelName enums #710

New issue

Closed

opened 2026-03-04 02:17:16 +03:00 by kerem · 1 comment

kerem commented

2026-03-04 02:17:16 +03:00

Owner

Originally created by @tigitz on GitHub (May 9, 2022).
Original GitHub issue: https://github.com/Seldaek/monolog/issues/1667

Parsing v3 code I feel like splitting the level name in it's own Enum doesn't bring any benefit while on the contrary it adds complexity and probably confusion for users.

They will have to constantly think about which one they should use in their context while in fact all they want is to use a Level. The name is just a property of the Level, the same way the int value is.

Could merging both enums into a single Level enum be considered an improvement ?

Something like this:

// Not a backed enum anymore
enum Level
{
    /**
     * Detailed debug information
     */
    case Debug;
   
    private const DEBUG_VALUE = 100;
    private const DEBUG_NAME = 'DEBUG';

    /**
     * @phpstan-param Level::*_NAME $name
     */
    public static function fromName(string $name): self
    {
        return match ($name) {
            self::DEBUG_NAME => self::Debug,
        };
    }

    /**
     * @phpstan-param Level::*_VALUE $value
     */
    public static function fromValue(int $value): self
    {
        return match ($value) {
            self::DEBUG_VALUE => self::Debug,
        };
    }

    /**
     * @phpstan-return Level::*_NAME
     */
    public function getName(): string
    {
        return match ($this) {
            self::Debug => self::DEBUG_NAME,
        };
    }

    /**
     * @phpstan-return Level::*_VALUE
     */
    public function getValue(): int
    {
        return match ($this) {
            self::Debug => self::DEBUG_VALUE,
        };
    }

Difference between current and supposedly improved usage:

Current version

// retrieved log level name as a string and test if it's debug
$logLevel = 'debug'
LevelName::from($logLevel) === LevelName::Debug

// should I compare the *name* or the level itself ? What's the best practice here ? 🤔 Maybe the level seems more appropriate
Level::fromLevelName(LevelName::from($logLevel)) === Level::Debug

// seems a bit verbose...

// retrieved log level as an int and test if it's debug
$logLevel = 100;
Level::from($logLevel) === Level::Debug

// now there's only one way to do the check, just because the initial value to represent a Level was an int instead of a string, seems not consistent.

//retrieving the name of a level
Level::Debug->toLevelName()->value;

Improved version

// retrieved log level name as a string
$logLevel = 'debug'
Level::fromName($logLevel) === Level::Debug

// retrieved log level name as an int
$logLevel = 100
Level::fromValue($logLevel) === Level::Debug

// Only 1 level concept to manage the level, api is clear on whether you need to represent a level from its name or its value, both can even be compared

Level::fromValue(100) === Level::fromName('debug')

//retrieving the name of a level
Level::Debug->getName();

Originally created by @tigitz on GitHub (May 9, 2022). Original GitHub issue: https://github.com/Seldaek/monolog/issues/1667 Parsing v3 code I feel like splitting the level name in it's own Enum doesn't bring any benefit while on the contrary it adds complexity and probably confusion for users. They will have to constantly think about which one they should use in their context while in fact all they want is to use a Level. The name is just a property of the Level, the same way the `int` value is. Could merging both enums into a single `Level` enum be considered an improvement ? Something like this: ```php // Not a backed enum anymore enum Level { /** * Detailed debug information */ case Debug; private const DEBUG_VALUE = 100; private const DEBUG_NAME = 'DEBUG'; /** * @phpstan-param Level::*_NAME $name */ public static function fromName(string $name): self { return match ($name) { self::DEBUG_NAME => self::Debug, }; } /** * @phpstan-param Level::*_VALUE $value */ public static function fromValue(int $value): self { return match ($value) { self::DEBUG_VALUE => self::Debug, }; } /** * @phpstan-return Level::*_NAME */ public function getName(): string { return match ($this) { self::Debug => self::DEBUG_NAME, }; } /** * @phpstan-return Level::*_VALUE */ public function getValue(): int { return match ($this) { self::Debug => self::DEBUG_VALUE, }; } ``` Difference between current and supposedly improved usage: *Current version* ```php // retrieved log level name as a string and test if it's debug $logLevel = 'debug' LevelName::from($logLevel) === LevelName::Debug // should I compare the *name* or the level itself ? What's the best practice here ? 🤔 Maybe the level seems more appropriate Level::fromLevelName(LevelName::from($logLevel)) === Level::Debug // seems a bit verbose... // retrieved log level as an int and test if it's debug $logLevel = 100; Level::from($logLevel) === Level::Debug // now there's only one way to do the check, just because the initial value to represent a Level was an int instead of a string, seems not consistent. //retrieving the name of a level Level::Debug->toLevelName()->value; ``` *Improved version* ```php // retrieved log level name as a string $logLevel = 'debug' Level::fromName($logLevel) === Level::Debug // retrieved log level name as an int $logLevel = 100 Level::fromValue($logLevel) === Level::Debug // Only 1 level concept to manage the level, api is clear on whether you need to represent a level from its name or its value, both can even be compared Level::fromValue(100) === Level::fromName('debug') //retrieving the name of a level Level::Debug->getName(); ```

kerem

2026-03-04 02:17:16 +03:00

closed this issue
added the
Feature
label

kerem commented

2026-03-04 02:17:18 +03:00

Author

Owner

@Seldaek commented on GitHub (May 9, 2022):

Yeah that sounds like an idea. The only downside is you don't have enum cases for level names anymore then, but in practice looking at the code now that it's all been converted we only ever refer to levelName->value, so Level->getName() would be just as good, and LevelName::VALUES could be Level::NAMES too.

I think I'll do the change.

@Seldaek commented on GitHub (May 9, 2022): Yeah that sounds like an idea. The only downside is you don't have enum cases for level names anymore then, but in practice looking at the code now that it's all been converted we only ever refer to levelName->value, so Level->getName() would be just as good, and LevelName::VALUES could be Level::NAMES too. I think I'll do the change.

kerem referenced this issue

2026-03-04 03:09:15 +03:00

[PR #710] [CLOSED] [WIP] Add ability to include exception's stack traces in Monolog\Formatter\JsonFormatter #1309