ramsey / uuid

:snowflake: A PHP library for generating universally unique identifiers (UUIDs).
https://uuid.ramsey.dev
MIT License
12.47k stars 501 forks source link

Deprecate `Uuid#setFactory()`, use dedicated factories for `FieldInterface` and for UUID creation #326

Open Ocramius opened 4 years ago

Ocramius commented 4 years ago

Problem-space

While working on #324, it became evident that the majority of complexity within this library comes from Uuid#setFactory(), which completely switches the behavior of the (supposedly) @psalm-pure operations, as well as the various Uuid::uuid1() through Uuid::uuid6() methods.

Solution

I suggest deprecating and discouraging usage of Uuid#setFactory() and Uuid#getFactory(), as well as deprecating all setters on UuidFactory itself, so that post-instantiation no internals of it can change.

After that, we can drop all this API for 5.0.0, allowing for major performance and design improvements within the core of the library.

Further scope

ramsey commented 4 years ago

After reading through #324, it became clear to me why setFactory() was so problematic, so I agree with deprecating it and getFactory(), as well as the setters on UuidFactory.

The FeatureSet is also extremely bloated

I agree, and I'd like to throw it out. This was originally a way to swap out components for testing, and it has grown significantly since introduced. Independent concrete implementations of UuidFactoryInterface sounds like the way to go.

Aggressive inlining can be applied to factory internals

I'd like to hear more about this. I'm not familiar with the term "inlining."

Ocramius commented 4 years ago

I'd like to hear more about this. I'm not familiar with the term "inlining."

Essentially copy-pasting static code to reduce the overhead from stack frame allocation. In static languages, the compiler does that implicitly, as an optimization.

While dependency injection does ease testing in this library, most of it is static code, if you assume only defaults.