Add Tx and Block pattern synonyms #2286
Conversation
DavidEichmann
requested review from
erikd,
intricate,
Jimbo4350,
mrBliss and
newhoggy
as code owners
| -- | A block consists of a header and a body containing transactions. | ||
| -- | ||
| pattern Block :: BlockHeader -> [Tx era] -> Block era | ||
| pattern Block header txs <- (getBlockHeaderAndTxs -> (header, txs)) |
There was a problem hiding this comment.
What are the alternatives to this? Why ViewPatterns ? Should we not be aiming for simple code?
There was a problem hiding this comment.
We're attempting to hide away internal representations in data Block era, and present a simplified representation here: a block is just a block header and some transactions. Eventually we should probably not export the other constructor of Block except is some sort of Internal module.
An alternative to this is to keep Block abstract and provide projection functions i.e. export getBlockHeaderAndTxs or similar.
There are some advantages to using a pattern synonyms. It indicates to users that this is in some sense a "complete" pattern (GHC is smart enough to see this is complete). The pattern also appears in the same place as constructors in the haddocks. This is more inline with a "closed world" design meaning users can just rely on pattern matching rather than having to search for projection functions. We also seem to use pattern synonyms already in various parts of cardano-api.
@dcoutts, perhaps you want to chime in on this?
There was a problem hiding this comment.
I agree that in general view patterns are horrid, and we don't use them generally.
To use (non-trivial) pattern synonyms (which is sensible here) we're forced to use view patterns. It's a very limited use. It doesn't infect anything else.
| @@ -6,6 +6,7 @@ | |||
| {-# LANGUAGE ScopedTypeVariables #-} | |||
| {-# LANGUAGE TypeApplications #-} | |||
| {-# LANGUAGE TypeFamilies #-} | |||
| {-# LANGUAGE ViewPatterns #-} | |||
There was a problem hiding this comment.
Why. I have never seen a case where using ViewPatterns has resulted in easier to read or easier to understand code.
There was a problem hiding this comment.
If you buy into exposing pattern synonyms, then we're forced to implement them with view patterns in order to use projection functions.
DavidEichmann
force-pushed
the
dcoutts/api-block-patterns
branch
from
7d8ad57 to
44b57a3
Compare
erikd
dismissed
their stale review
I should not comment on PRs at the end of a 12 hour day
|
bors merge |
|
Build succeeded: |
No description provided.