Change calls to method toArray() from self::toArray() to static::toArray()

[lorenzomar]

See #20
The idea is to allow subclasses to provide a data set different from constants list. The default implementation still use constants but you can extend it to use different data source; so for instance if you have data stored in a DB you can still use your specific Enum in type hinting, but fetch data from DB.

[mnapoli]

I'm afraid the change to __callStatic would have a performance impact (can be called a lot in some applications).

[lorenzomar]

It is an interesting point to explore. I will do some performance test and share the results.

[mnapoli]

Wouldn't it be better to introduce an EnumInterface? I understand the need (e.g. enum stored in db or whatever) but I'm wondering if it's worth complicating even more the current class (both for performances and maintenance). Implementing a separate class for the "DB enum" would be so easy that I'm not sure inheritance is the appropriate solution.

[lorenzomar]

It is a possibility, although in that case a lot of common logic would be lost.
Maybe a better way is to introduce an interface and to provide a base abstract class that let the client with special needs the task to implement toArray() and __callStatic(). The old Enum class will extend the abstract one leaving intact its previous implementation so that no one has to worry about the changes.

[lorenzomar]

I did a performance test. You can find repository here: https://github.com/lorenzomar/php-enum-test.

Results - original implementation:
__callStatic:
Duration (ms): 1305ms
Average Duration (ms): 0.0435ms
Memory (bytes): 786432

__constructor:
Duration (ms): 735ms
Average Duration (ms): 0.0245ms
Memory (bytes): 786432

Results - edited implementation:
__callStatic:
Duration (ms): 1609ms
Average Duration (ms): 0.053633333333333ms
Memory (bytes): 786432

__constructor:
Duration (ms): 733ms
Average Duration (ms): 0.024433333333333ms
Memory (bytes): 786432

[mnapoli]

Having an abstract class + an interface would cause loading 3 files instead of one :/ Even though it may sounds ridiculous I've definitely seem the performance matter (that's why I removed it from PHP-DI 5 for example), and I'm not sure it's really worth it… (the code is really simple, I'm fine with such code duplication honestly)

[lorenzomar]

Take a look at the performance tests. It seems really good. I did the test on 30.000 initializations of a class containing 300 constants. You can obtain an average difference of 1ms doing about 3.000.000 of init (It is a rough calculation :) ).
Maybe the first hypothesis is a good one.

[mnapoli]

OK let's merge that then. Last suggestion, change that:

if (static::isValidKey($name)) {
    $array = static::toArray();
    return new static($array[$name]);
}

into that:

$array = static::toArray();
if (isset($array[$name])) {
    return new static($array[$name]);
}

?

[lorenzomar]

In this case i think that duplicate the logic used to check if a key is valid isn't a problem.
I'll push the change in a few minutes.

[mnapoli]

👍

[lorenzomar]

Done. I'm already using the new feature (my repo until the pull request isn't merged) in this project https://github.com/valueobjects/geography/tree/master/src/Country where i have a DataProvider used by multiple Enum implementation (Iso31661Alpha2Code, Iso31661Alpha3Code, PhonePrefix). I provide a file based data provider (DefaultDataProvider) but applications that already have a data set in some places (DB, API, ...) can do a custom implementation of DataProviderInterface and use their data source.

PS. The project is still a working in progress, so... no test, no docs and a little bit of mess all around :)

[mnapoli]

Thanks! I tagged 1.4.1