search

korken89 / smlang-rs

A State Machine Language DSL procedual macro for Rust
Apache License 2.0
202 stars 28 forks source link

Please remove PartialEQ for States and Events #21

Closed videni closed 2 years ago

videni commented 3 years ago

The event data and state data sometimes may be very complex, in my case , I have an OrderAggregate object which consist of Product, Promotion, etc, they also are big object. so it is not practical to implment PartialEQ.

removing the two traits, the end user still can compare use this way compare-enums-only-by-variant-not-value

Yatekii commented 3 years ago

Well, just comparing the variant does not suffice for comparing for equality.

korken89 commented 3 years ago

Hi, I remember there were issues with not having PartialEq, but I do not quite remember what it was. If you feel like giving it a try you can remove it and see what breaks, and we can take it from there.

videni commented 3 years ago

yes, there will be an error, any other way please? the use case is simple, our event data is an aggregate of other aggregates, impossible to implement PartialEq.

korken89 commented 3 years ago

Could you paste the error here? I'll see if it can be fixed easily.

dzimmanck commented 2 years ago

Removing the PartialEq causes a number of tests to fail that expect to use simple assertion checks for simple enums based on "==". I think a good solution here would be to have the proc macro generate the PartialEq trait definitions based on just the enum discriminant. This way non of the tests need to be changed and we don't break anyone's code that was relying on this.

@korken89 , if you don't mind I will take a crack at this.

dzimmanck commented 2 years ago

Created a pull request to resolve.

pull request #25

dzimmanck commented 2 years ago

Resolved with PR #25