haskell-graphql / graphql-api

Write type-safe GraphQL services in Haskell
BSD 3-Clause "New" or "Revised" License
406 stars 35 forks source link

Simplify code by using DuplicateRecordFields and NamedFieldPuns #206

Open zhujinxuan opened 5 years ago

zhujinxuan commented 5 years ago

Recently I am working on https://github.com/haskell-graphql/graphql-api/pull/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 = node@AST.Node {_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.

jml commented 5 years ago

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 commented 5 years ago

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

zhujinxuan commented 5 years ago

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