Make deduplication configurable from env

[greg0ire]

When in the context of PHPUnit tests, using a bootstrap file is not very convenient.

Prompted by doctrine/collections#472

Disclaimer: I used opencode + Claude Sonnet for this.

[greg0ire]

@mpdude please review

[mpdude]

What is this doing here? Flipping self::$deduplication to some value as a side effect?

[greg0ire]

🤔 what do you mean "a side effect"? If this piece of code is reached, then it means there is either no env var defined, or it is neither false nor 0. In that case, we should fallback to the value before this PR, which was true.

[mpdude]

self::$deduplication ?? self::getDeduplicationFromEnv() is a repeating pattern. I'd make a static function for it that returns self::$deduplication ?? ...whateverDerivedFromEnv...

[greg0ire]

self::$type ?? self::getTypeFromEnv(); is also a repeating pattern (even more repeating in fact), so although I had the same idea as you, I thought I would be consistent with the existing code

[mpdude]

Disclaimer: I used opencode + Claude Sonnet for this.

That might explain 🤣

[greg0ire]

That might explain 🤣

Well I actually reworked getDeduplicationFromEnv() myself, so if you find it a bit weird, I think it's my fault :P

Here is what Claude went with:

        if ($envValue === 'false' || $envValue === '0') {
            self::$deduplication = false;
        } else {
            self::$deduplication = true;
        }

        return self::$deduplication;

[mpdude]

So would you want to change the state of self::$deduplication from within that getter, so that the env var is evaluated only once?

I see, as a kind of lazy initalization and avoiding the performance overhead of re-evaluating it every time...

[greg0ire]

Yes, the performance overhead, and also, from a conceptual point of vue, env variables should be considered immutable. One should assume they are never going to change during the lifetime of a program.