Rector: [idea] Refactoring enum value objects

[JanMikes]

Using this rector:

services:
    Rector\Rector\Dynamic\ValueObjectRemoverRector:
        $valueObjectsToSimpleTypes:
            'OdbavTo\Domain\Model\Ticket\Input\GuestField': 'string'

Preview of what can/can not be done:

• [x] foo(GuestField $guestField) -> foo(string $guestField)

• [x] /** @var GuestField|null */ -> /** @var string */
  @TomasVotruba this seems to be a bug as it introduces bc break -> it was nullable before, now in annotation is only string instead of null|string

• [x] public function guestField(): ?GuestField -> public function guestField(): ?string @TomasVotruba rector did not find and fix this one

• [x] public function updateGuestField(?GuestField $guestField): void ->public function updateGuestField(?string $guestField): void
  @TomasVotruba rector seems to have problems with nullable, as this one was not fixed

---

Edited by @tomasvotruba

Another Refactoring

These are not related to valueObjects, but rather custom Enums.

• $guestField->equals($ticketInputValue->guestField()) -> $guestField === $ticketInputValue->guestField()
  Any idea how this could be done via rector? I am thinking about regex find/replace right now, it should not be hard, but the idea is to teach rector how to do this, let's change world together

• GuestField::LAST_NAME() -> GuestField::LAST_NAME
  This one can be easily done via regex find/replace

[TomasVotruba]

I'll look on this tomorrow. Thanks for before/after expected examples. They're most helpful :bowing_man:

[TomasVotruba]

GuestField::LAST_NAME() -> GuestField::LAST_NAME
This one can be easily done via regex find/replace

What is this? Capital static method name to constant?

GuestField will be removed completely and replaced by string, so I have no idea what you mean.

[TomasVotruba]

$guestField->equals($ticketInputValue->guestField()) -> $guestField === $ticketInputValue->guestField()
Any idea how tnhis could be done via rector? I am thinking about regex find/replace right now, it should not be hard, but the idea is to teach rector how to do this, let's change world together

This one will be a little bit tricky, but it can be done. What are possible method names, only equals?

[JanMikes]

What is this? Capital static method name to constant?

Yeah. The GuestField is enum, you can check it here.

TLDR, GuestField::LAST_NAME() is static factory for GuestField with value of lastName, and i want to replace it with simply constant of GuestField class.

[JanMikes]

This one will be a little bit tricky, but it can be done. What are possible method names, only equals?

Yeah, it would be nice to have method name dynamic as some other value objects can use isSameAs() or isSame().

Another one i found is:

public function isNull(): bool
    {
        return $this->value() === null;
    }

Before:

$object->isNull()

After:

$object === null

[TomasVotruba]

1. So it's the same class name but different namespace? If so, please complete those namespaces above, so it's clear.

2. Ok, got it :)

[JanMikes]

It is same class. It is class method->constant usage on same class.
We do not have rector MethodToConstantRector yet, right?

Before refactoring:

/**
 * @method static LAST_NAME()
 */
class GuestField extends Enum
{
    public const LAST_NAME = 'lastName';
}

After:

class GuestField
{
    public const LAST_NAME = 'lastName';
}

[TomasVotruba]

But GuestField class is removed from the code and value used directly. That purpose of this while operation.

[JanMikes]

This class does not get removed completely. I want it to remain as empty class only with constants. Does it make sense is it some kind of antipattern?

[TomasVotruba]

Looking at that that another operation. We can look on that later, but now let focus on removing value object only.

When it's finished, we can meetup and look on others case it's related to. What do you think?

[JanMikes]

Sure thing 👍

[adaamz]

What is goal of removing enums and replacing them with scalars?

[TomasVotruba]

@CZechBoY Honza gave a funny talk with great examples as an answer:
https://www.facebook.com/pehapkari/videos/vl.1779501342352698/1587929124590225/?type=1

[JanMikes]

Using many ENUMs is negatively affecting doctrine performance in our project and i was not able to find any good solution for that.

In that app performance is critical.

[JanMikes]

Though i really like the idea as it allows strict type hinting.

foo(GuestField $field) vs foo(string $guestField) -> you can basically pass anything to that method.

If here is any good path to optimize it for performance, i am listening 😄 we are using marc-mabe/php-enum.

@CZechBoY Though, i was thinking about this approach in entities, which would solve the biggest overhead of doctrine custom types, but i don't like the php code and retyping in entities:

/** @var string */
private $guestField;


public function getGuestField(): GuestField
{
    return GuestField::byValue($this->guestField);
}


public function setGuestField(GuestField $field): void
{
    $this->guestField = $field->getValue();
}

Any tips are very welcomed and appreciated!

[TomasVotruba]

After more digging into this I recommend using manual PhpStorm refactoring for enum changes. They're very individual for every project and thus not a good material for Rector.

On the other hand, ValueObjectsRemover was fixed for all mentioned issues in #354 and now covers docblocks as well :tulip: It needs real-life testing, since docblock don't have any useful parsing tool right know (like php has php-parser). Let me know feedback if that works for you.

[JanMikes]

@TomasVotruba thank you very much! Next week i will give it a try and let you know with next wave of feedback 😉

[jeroenherczeg]

@CZechBoY Honza gave a funny talk with great examples as an answer:
https://www.facebook.com/pehapkari/videos/vl.1779501342352698/1587929124590225/?type=1

Is this presentation available in English somewhere?

[JanMikes]

Hi @jeroenherczeg unfortunately nope, but if you want I can translate it and upload it to Slideshare. It should not take much time 😉 (I suppose we are talking about the presentation itself, not about the talk 😄 )

[jeroenherczeg]

I had the same question "What is goal of removing enums and replacing them with scalars?"
Do you think I would get a good answer from the translated slides? Or could you give me the TLDR of the presentation? :)

[JanMikes]

Sure! The main thing I was worried about was performance. Because we used custom doctrine types for each value object, the impact was huge. In slides, there is a comparison before and after (primitives vs value object with custom type).

I was comparing it on production REST API, hydrating thousands of entities.