Skip to content

Record instances of Show, Eq, Ord etc #154

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

Closed
LiamGoodacre opened this issue Jan 22, 2018 · 6 comments
Closed

Record instances of Show, Eq, Ord etc #154

LiamGoodacre opened this issue Jan 22, 2018 · 6 comments

Comments

@LiamGoodacre
Copy link
Member

Once we have moved RowToList (etc) to Prim, we can define these instances for Record.

@hdgarrood
Copy link
Contributor

👍 I would also like to include semigroup, monoid, semiring, ring, and commutative ring instances, which behave similarly to the existing Tuple ones. I don’t think we can do EuclideanRing because we won’t have an integral domain - for instance, (a,0) * (0,b) = (0,0) so we have zero divisors.

@LiamGoodacre
Copy link
Member Author

We should discuss whether Ord (Record _) (and possibly others) makes sense given the key sorting from RowToList.

@i-am-tom - For now, only implement those in which the ordering isn't a significant factor. Show (Record _) is fine though.

@hdgarrood
Copy link
Contributor

Can you remind me what the key sorting is? As Phil pointed out elsewhere I think it might be best not to provide Ord, as people will probably assume that if they use a type synonym like

type Person = { name :: String, age :: Int }

then they might expect lexicographical ordering based on the order fields are listed in the type synonym.

@LiamGoodacre
Copy link
Member Author

Yes, it's not (can't be) the order the keys appear in the source (because they can appear in different orders and are still the same type). We could still provide Ord but the keys will always be in sort order - because that's what we get from RowToList. Also the derive for Ord currently does this sorting too - so it would be consistent with that. However, Phil mentioned that he's not sure this is necessarily a good default for Record.

There could separately exist an newtype for Records that is indexed by the keys in a specific order.

@hdgarrood
Copy link
Contributor

In that case, shall we leave Ord (Record r) out for now and come back to it later if it comes up again?

@garyb
Copy link
Member

garyb commented May 13, 2018

Closing as we have this in the 0.12 branch now 🎉

@garyb garyb closed this as completed May 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants