-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement enums #1
Conversation
src/IconBuilder.php
Outdated
|
||
class IconBuilder | ||
{ | ||
private array $allowedSizes = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was a Enums\Size not introduced?
Enums can also be used for verifying incoming data.
Say for example your ->size(string $size) method could be typed for (Size|string $size)
if(! is_string($size)) {
return $this->addClass('fa-', $size->value); // we know we have a valid value
}
$size = Size::tryFrom($size) ?? Size::Default->value; // Where Default can be an Enum constant alias pointing to whichever size you want to be the default.
public final const Default = '1x';
case 1x = self::Default;
At which time, instead of throwing that exception you could provide real time user feedback that they have entered an incorrect value and that the default will be used along with providing a list of possible values, provided via the Enum without the need for a trip to a db.
Dont get me wrong. The current code works :) The point of Enums is to always work with known types and values :) I have not benchmarked it but I would guess that tryFrom is faster than in_array. I should probably bench that come to think of it.
|
||
protected function getSegment(Type $type): string | ||
{ | ||
return match ([$this, $type]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty creative. Actually solves an issue that I was facing in some code myself.
src/Traits/HasCollection.php
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These really should match the naming convention for the Interface contracts that they honor.
composer.json
Outdated
@@ -16,7 +16,7 @@ | |||
], | |||
"autoload": { | |||
"psr-4": { | |||
"Bugo\\FontAwesomeHelper\\": "src/FontAwesomeHelper/" | |||
"Bugo\\FontAwesomeHelper\\": "src/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably would have just named that FAHelper. But I hate mile long namespaces :P
No description provided.