Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to provide extra values to EnumCase attribute #170

Merged
merged 1 commit into from Jan 27, 2022
Merged

Conversation

ogizanagi
Copy link
Member

@ogizanagi ogizanagi commented Jan 13, 2022

@IonBazan
Copy link
Contributor

Makes sense to me 💯
Would that be also possible to add a method to get the actual attribute instance to allow extending EnumCase attribute while still benefiting from the cache?

@ogizanagi
Copy link
Member Author

ogizanagi commented Jan 13, 2022

Would that be also possible to add a method to get the actual attribute instance to allow extending EnumCase attribute while still benefiting from the cache?

Might be. But I think your use-case might be covered by:

/**
 * @final
 */
#[\Attribute(\Attribute::TARGET_CLASS_CONSTANT)]
class RenderableCase extends EnumCase
{
    public function __construct(string $label, string $color, string $icon)
    {
        parent::__construct($label, extra: compact('color', 'icon'))
    }
}

while still requiring only getExtra, if that's only about more scoped PHP attributes, isn't it?

@IonBazan
Copy link
Contributor

IonBazan commented Jan 13, 2022

My concern is type safety. With this extra: ['something' => 'test'] we are not sure what items and what types are stored there. If we could have a method that returns all attributes that extend EnumCase, we could store and retrieve any kind of extra information with ease and type safety.

Something like:

/**
 * @return EnumCase[]
 */
public function getAttributes(): array
{
    // ...
}

public function getColor(): ?string
{
    foreach ($this->getAttributes() as $attribute) {
        if ($attribute instanceof HasColor) {
            return $attribute->color;
        }
    }

    return null;
}

We could combine both approaches - for quick results, developers might use this extra field, but for more sophisticated logic, they might want to use the helper that returns the attributes assigned to the case.

@ogizanagi
Copy link
Member Author

ogizanagi commented Jan 13, 2022

But the example in #170 (comment) is enough to provide type safety to me (unless you use both EnumCase and RenderableCase in your code base for the same needs, but that's not the point of creating a dedicated attribute then).

@ogizanagi ogizanagi marked this pull request as ready for review January 25, 2022 16:04

trait EnumCaseAttributesTrait
{
private function getEnumCaseAttribute(): ?EnumCase
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(methods are private, but can be used by any userland implementation using this trait, as not marked @internal)

@ogizanagi ogizanagi force-pushed the enum-case-extras branch 2 times, most recently from 209bacd to 028cf9d Compare January 25, 2022 16:10
@ogizanagi ogizanagi added this to the 2.x milestone Jan 25, 2022
@ogizanagi ogizanagi added this to In Progress in v2.0 via automation Jan 25, 2022
@ogizanagi ogizanagi merged commit 1a1c3d6 into 2.x Jan 27, 2022
@ogizanagi ogizanagi deleted the enum-case-extras branch January 27, 2022 09:15
v2.0 automation moved this from In Progress to Done Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
v2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants