Skip to content

Simplify code by using DuplicateRecordFields and NamedFieldPuns #206

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

Open
zhujinxuan opened this issue Nov 2, 2018 · 3 comments
Open

Simplify code by using DuplicateRecordFields and NamedFieldPuns #206

zhujinxuan opened this issue Nov 2, 2018 · 3 comments

Comments

@zhujinxuan
Copy link

zhujinxuan commented Nov 2, 2018

Recently I am working on #205, and I find that I have to manually add _ everywhere for adding position infomation. And we can see quite a lot repetitive code like (in Validation.hs)

-- | Get the selection set for an operation.
getSelectionSet :: Operation value -> SelectionSetByType value
getSelectionSet (Query _ _ ss) = ss
getSelectionSet (Mutation _ _ ss) = ss

However, we can simplify them with DuplicateRecordFields and NamedFieldPuns. For example, we can have the following code in Validation.hs that:

{-# DuplicateRecordFields #-}

data Operation value
  = Query {
       _ss :: VariableDefinitions
      , _dv :: (Directives value) 
      , getSelectionSet :: (SelectionSetByType value)
  | Mutation {
      _ss :: VariableDefinitions 
      , _dv :: (Directives value)
      , getSelectionSet :: (SelectionSetByType value)
   } deriving (Eq, Show)

Furthermore, with {-# NamedFieldPuns #-}, we can simplify some other code from

    splitOps (AST.Query node@(AST.Node maybeName _ _ _ _) _) = Right (maybeName, (Query, node))

to

{-# NamedFieldPuns #-}
    splitOps AST.Query {_node = [email protected] {_name = maybeName}}  = Right (maybeName, (Query, node))

Personally, I think by that we can make code easier to write without carefully counting the number of _s, and take advantage with other language extensions of Records when developing with graphql.

@teh teh added the enhancement label Nov 3, 2018
@jml
Copy link
Collaborator

jml commented Nov 6, 2018

Very strongly in favour of this. If I remember rightly, when we wrote this, @teh and I disagreed about whether we should prefer pattern matching (as described above) or field accessor functions. We only latched on to NamedFieldPuns later on in the piece.

@teh
Copy link
Collaborator

teh commented Nov 7, 2018

I can't even remember that discussion but it does seem like a sensible change!

@zhujinxuan
Copy link
Author

@teh , @jml Great. I will do that change after #205 is finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants