Skip to content
New issue

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

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

Already on GitHub? # to your account

Updates to Avram Enums : ==, ===, enum constants #566

Merged
merged 4 commits into from
Dec 22, 2020

Conversation

robacarp
Copy link
Contributor

@robacarp robacarp commented Dec 17, 2020

This implements == and === methods on avram_enum generated structures, and also implements explicit constant accessors to decrease the difference between Enum and avram_enum.

Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

I love this. Question.... Should we also add in the threequals ===? I think case uses that, right?

Also provides named constants which reflect the underlying enum values
@robacarp robacarp changed the title Implements == on Avram Enums Updates to Avram Enums : ==, ===, enum constants Dec 17, 2020
@akadusei
Copy link
Contributor

Using a struct for the enum wrapper instead of a class should solve the core problem this PR seeks to resolve.

An instance of a class is a different object from another instance of the same class. An instance of a struct will always be equal to another instance of the same struct.

@robacarp
Copy link
Contributor Author

@akadusei By converting the enum wrapper to a struct I was indeed able to remove the explicit definition of the #== operator. Honestly I don't know all the implications of this kind of change, though from all my reading a struct is better than a class wherever you can. I don't know what the knock-on effects of this will be in developer applications.

I added it here a a separate commit which can be rebased out if undesired.

def ==(other : {{ enum_name }}) : Bool
self.enum == other.enum
end

delegate :===, to_s, to_i, to: @enum
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need to delegate === here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, the case equality tests fail without it.

@akadusei
Copy link
Contributor

Looks good. Apps should not have any issues, unless they are reopening the class (now struct) for some reason.

Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

I haven't used the enum yet, but I think this looks good. Only way to really know is get it in the hands of people!

end

class {{ enum_name }}
struct {{ enum_name }}
Copy link
Member

Choose a reason for hiding this comment

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

nice! 👍

@jwoertink jwoertink merged commit 26ff478 into luckyframework:master Dec 22, 2020
@robacarp robacarp deleted the enum_equality branch December 22, 2020 22:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants