3
\$\begingroup\$

I am writing a Tetris game while learning Haskell and I'm trying to make the following code more concise and easier to read. How can I improve it?

Note that the function is not finished yet (There is no check for complete lines, for instance); I currently want to improve its readability.

updateGame :: IORef InputsData -> IORef GameState -> IO()
updateGame keysIORef gameIORef = do
    game <- readIORef gameIORef
    keys <- readIORef keysIORef
    newTime <- getCurrentTime
    elapsed <- getMilliseconds $ diffUTCTime newTime (time game)

    -- Updating current time
    modifyIORef gameIORef (\g -> g {time = newTime})
    game <- readIORef gameIORef

    -- Updating keys cooldown
    modifyIORef gameIORef (\g -> g {keyCooldowns = updateCooldowns (keyCooldowns game) elapsed})
    game <- readIORef gameIORef

    -- Processing side movements
    when (not $ isOnCooldown (keyCooldowns game) (SpecialKey KeyRight)) $ do
        if right keys
          then when (canTetrominoMove (currentTetromino game) (Helpers.R) (tank game)) $ do
            modifyIORef gameIORef (\g -> g {currentTetromino = (currentTetromino g) {x = x (currentTetromino g) + 1}})
            modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (SpecialKey KeyRight) 100})
          else if left keys
            then when (canTetrominoMove (currentTetromino game) (Helpers.L) (tank game)) $ do
              modifyIORef gameIORef (\g -> g {currentTetromino = (currentTetromino g) {x = x (currentTetromino g) - 1}})
              modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (SpecialKey KeyRight) 100})
            else return ()
    game <- readIORef gameIORef

    -- Processing tetromino rotations
    when (not $ isOnCooldown (keyCooldowns game) (Char 'z')) $ do
        if xKey keys
          then when (canTetrominoRotate (currentTetromino game) (Helpers.R) (tank game)) $ do
            modifyIORef gameIORef (\g -> g {currentTetromino = rotateTetromino (currentTetromino g) Helpers.R})
            modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (Char 'z') 200})
          else if zKey keys
            then when (canTetrominoRotate (currentTetromino game) (Helpers.L) (tank game)) $ do
              modifyIORef gameIORef (\g -> g {currentTetromino = rotateTetromino (currentTetromino g) Helpers.L})
              modifyIORef gameIORef (\g -> g {keyCooldowns = addCooldown (keyCooldowns game) (Char 'z') 200})
            else return ()
    game <- readIORef gameIORef

    -- Updating current tetromino falling time
    modifyIORef gameIORef (\g -> g {tetrominoFallingTime = (tetrominoFallingTime g) - elapsed})
    game <- readIORef gameIORef

    -- Making current tetromino fall & restarting current tetromino falling time
    when (tetrominoFallingTime game < 0) $ do
        modifyIORef gameIORef (\g -> g {tetrominoFallingTime = (tetrominoFallingTime g) + getTetrominoFallingSpeed (difficultyLevel g)})
        if canTetrominoFall (currentTetromino game) (tank game)
            then do
                modifyIORef gameIORef (\g -> g {currentTetromino = (currentTetromino g) {y = y (currentTetromino g) - 1}})
            else do
                modifyIORef gameIORef (\g -> g {tank = addTetrominoToTank (currentTetromino game) (tank game)})
                let (currentTetromino', tetrominoBag', rndGen') = drawTetrominoFromBag (rndGen game) (tetrominoBag game)
                modifyIORef gameIORef (\g -> g {currentTetromino = currentTetromino', rndGen = rndGen', tetrominoBag = tetrominoBag'})

    addTimerCallback 1 $ updateGame keysIORef gameIORef
    postRedisplay Nothing
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

You should separate IO actions from pure code. In your case the principal task of updateGame is to create the new game state given input data, and for this no IO is necessary. The fact that the state is stored and modified in an IORef.

It's not just the matter of style and code separation. Calling modifyIORef multiple times in the function means that you can easily get race conditions, if there is another thread working with the game state. It would get or modify an inconsistent state.

So my suggestion would be to split it into two parts - one a pure function, and another that does all the necessary IO:

updateGame :: UTCTime -> InputsData -> GameState -> GameState
updateGame time input game = ...


updateGameIO :: IORef InputsData -> IORef GameState -> IO ()
updateGameIO modifyIORef gameIORef = step
  where
    step = do
      time <- getCurrentTime

      keys <- readIORef keysIORef
      modifyIORef gameIORef (updateGame time keys)

      addTimerCallback 1 step
      postRedisplay Nothing

This way, you ensure that the game state is updated atomically, and keep the main function pure.


Updating a field in a complex data type is a common task, and it can be greatly simplified using the lens library. Instead of

\g -> g {keyCooldowns = updateCooldowns (keyCooldowns game) elapsed}

you let lens generate so-called lenses for your data type, so for example keyCooldowns won't be of type GameState -> Something, but Lens' GameState Something, and then the function that modifies the field inside GameState would be

over keyCooldowns (flip updateCooldowns elapsed)

See A Little Lens Starter Tutorial

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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