3
\$\begingroup\$

Is there a nicer way to write the maybeWhen portion of displayConnectionAction ? I want to conditionally display an Action depending on whether or not the player is connected.

This bit of code is lifted from a wider system where there are multiple calls like displayConnectionAction (conditional actions built up by a Reader GameState), so I am not after any feedback around things like the Reader being overkill etc, more the implementation of the function displayConnectionAction, the actual code has 12+ functions like this, all generating Maybe Actions based off various bits of logic.

data Action = Action
  { actionLabel :: Text
  , actionUrl :: Text
  , actionPlayer :: Int
  }

data GameState = GameState
  { currentPlayer :: Player
  }

data Player = Player
  { playerId :: Int
  , playerName :: Text
  , playerConnected :: Bool
  }

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = maybeWhen not <$> isConnected <*> action
  where
    action =
      Just . Action "Connect" "/connect" <$> currentPlayerId

    isConnected =
      asks (playerConnected . currentPlayer)

    currentPlayerId =
      asks (playerId . currentPlayer)

maybeWhen :: (a -> Bool) -> a -> Maybe b -> Maybe b
maybeWhen f a mb =
  if f a then mb else Nothing
\$\endgroup\$
4
\$\begingroup\$

It always looks a little odd when one has to 'invent' something that seems like it ought to be a common, built-in expression, so I can understand why you ask.

Please interpret any of the following as nothing but my humble attempt to share what I know about Haskell programming. I don't consider myself a Haskell expert, and I don't know if other people would consider any of the following idiomatic...

Simplify maybeWhen

Is it necessary to pass both a predicate and a value (a) when the value itself turns out to be a Boolean value?

Would it make sense to simplify maybeWhen to something like the following?

maybeWhen' :: Bool -> Maybe b -> Maybe b
maybeWhen' b mb =
  if b then mb else Nothing

This would enable you to rewrite displayConnectionAction like this:

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = maybeWhen' <$> (not <$> isConnected) <*> action
  where
    action =
      Just . Action "Connect" "/connect" <$> currentPlayerId

    isConnected =
      asks (playerConnected . currentPlayer)

    currentPlayerId =
      asks (playerId . currentPlayer)

All of the function body (from where and down) is the same; only the composition is different.

Notice that it first maps isConnected with not, and the applies the projected value and action to the simplified maybeWhen' function.

maybeWhen' is just a composition

Instead of writing a function like maybeWhen', you can compose it from bool from Data.Bool:

Prelude Data.Bool> :t flip (bool Nothing)
flip (bool Nothing) :: Bool -> Maybe a -> Maybe a

Plugging that into displayConnectionAction means that you can now write it like this:

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = flip (bool Nothing) <$> isConnected <*> action
  where
    action =
      Just . Action "Connect" "/connect" <$> currentPlayerId

    isConnected =
      asks (playerConnected . currentPlayer)

    currentPlayerId =
      asks (playerId . currentPlayer)

Again, all code below where is the same. Notice, however, that you no longer need any helper function.

Or, instead of using the flip function, you could just flip the arguments:

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = bool Nothing <$> action <*> isConnected
  where
    action =
      Just . Action "Connect" "/connect" <$> currentPlayerId

    isConnected =
      asks (playerConnected . currentPlayer)

    currentPlayerId =
      asks (playerId . currentPlayer)

Notice that in this version, action and isConnected has changed places.

All this, however, relies on the simplification that in this particular example, the a in maybeWhen turns out to be Bool. Perhaps that's not always the case...

Use do-notation

Personally, I think this would be more readable if you rewrite the function using do notation. There are definitely Haskell programmers who consider do notation a crutch that has 'nothing to do with functional programming', but I don't share that sentiment.

Since Reader is a Monad, you could refactor the function to this implementation:

displayConnectionAction :: Reader GameState (Maybe Action)
displayConnectionAction = do
  currentPlayerId <- asks (playerId . currentPlayer)
  let action = Just . Action "Connect" "/connect" $ currentPlayerId
  isConnected <- asks (playerConnected . currentPlayer)
  if not isConnected
    then return action
    else return Nothing

Instead of having various Reader GameState values that you need to figure out how to combine, a variable like currentPlayerId is now simply an Int, and isConnected is simply a Bool.

Ultimately, though, if you often find yourself short-circuiting Bool values to Nothing, you may want to consider using guard, when, and unless with the Maybe monad. In this case, though, this would require you to use a monad transformer so that you can either stack your Reader inside MaybeT, or stack Maybe inside ReaderT.

\$\endgroup\$
1
\$\begingroup\$

I'll stand on Mark Seemann's shoulders:

displayConnectionAction = withReader currentPlayer $ do
  connected <- asks playerConnected
  if connected
    then return Nothing
    else asks $ Just . Action "Connect" "/connect" . playerId

I wouldn't qualify field names with what records define them, it leads to redundant name dropping in compositions.

MaybeT (or equivalently ReaderT r Maybe) can do us one better:

displayConnectionAction =
  withReader currentPlayer $ runMaybeT $ do
    False <- asks playerConnected
    asks $ Action "Connect" "/connect" . playerId

With more context, next steps may include checking whether Action is consumed in only one place, and if so whether we can produce what it's turned into immediately. If a connected player means a presence of player data which would be obtained by the connection action, <|> could capture that pattern so we can get rid of that stinky boolean handling.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.