I won't restate the problem in full, but in a nutshell you have to parse a file with a "direction" and a "magnitude", for instance:

forward 8
forward 3
down 8
down 2
up 1

forward affects your horizontal position and up and down your vertical position. There are no other directions. At the end, you're asked to multiply your final coordinates.

And this is my solution. It finds the right solution, but can it be improved? Anything that screams "not idiomatic Haskell"? I'm not fond of the toTuple function... but I had no idea how to do parse them better.

data Direction = Up | Down | Forward
instance Show Direction where                                                                              
    show Up = "up"                                                                                      
    show Down = "down"                                                                                  
    show Forward = "forward"                                                                          
instance Read Direction where                                                                           
    readsPrec _ "up" = [(Up, "")]                                                                          
    readsPrec _ "down" = [(Down, "")]                                                                      
    readsPrec _ "forward" = [(Forward, "")]                                                                
type HorizontalPosition = Int                                                                              
type VerticalPosition = Int                                                                           
data Position = Position {                                                                                 
    hPos :: HorizontalPosition,                                                                         
    vPos :: VerticalPosition }                                                                             
    deriving Show                                                                                       
move :: Position -> Direction -> Int -> Position    
move p Up n      = Position (hPos p) (vPos p - n)    
move p Down n    = Position (hPos p) (vPos p + n)                                                     
move p Forward n = Position (hPos p + n) (vPos p)                                                          
toTuple :: [String] -> (Direction, Int)                                                               
toTuple [d, n] = (read d :: Direction, read n :: Int)                                                   
main = do                                                                                               
    content <- readFile "input02"                                                                       
    let moves = map (toTuple . words) $ lines content                                                      
        position = foldl (\p (d, n) -> move p d n) (Position 0 0) moves                                                                                                                       
    putStrLn $ show $ hPos position * vPos position                                                                                                                                           

1 Answer 1

  • The general preference is for Show/Read instances that produce and parse (text that looks like) Haskell code. The expectation is that you should be able to paste the result of show x into code and get the same value out. That's to say: I don't like how you define a "weird" Read Direction instance. Also note that it "requires" you to define a compatible Show, which simply isn't contributing to solving the problem. Just define a freestanding parser.

    readDirection :: String -> Maybe Direction
    readDirection "up"      = Just Up
    readDirection "down"    = Just Down
    readDirection "forward" = Just Forward
    readDirection _         = Nothing
  • Using two different type aliases for the components of Position is pointless, since they are constrained to be the same by move, and you're also forced to have "bare" Int in move and direction. If you want an alias, make one that can be used in all the parts that are "connected" (so that a change to the alias doesn't immediately break the program.)

    type Coordinate = Int
    -- see note about foldl later; using foldl' does almost nothing for a record unless it has strict fields
    data Position = Position { hPos :: !Coordinate, vPos :: !Coordinate }
  • You may as well define

    data Command = Command { commandDirection :: Direction, commandDistance :: Coordinate }
    move :: Command -> Position -> Position -- writing it this way allows thinking of move :: Command -> (Position -> Position)
    move (Command Up      n) p = p { vPos = vPos p - n } -- record update syntax is a thing!
    move (Command Down    n) p = p { vPos = vPos p + n }
    move (Command Forward n) p = p { hPos = hPos p + n }

    instead of using a tuple/two separate arguments. (This also removes the need to clumsily adapt between those two representations.)

  • You can now use the monadic/applicative nature of Maybe to define a parseLine nicely, instead of toTuple. Note that we should avoid runtime errors on invalid input (so no partial patterns allowed and no read allowed).

    parseLine :: String -> Maybe Command
    parseLine line = do
      [dir, n] <- return $ words line -- partial match left of <- in Maybe simply short circuits to Nothing instead of erroring
      Command <$> readDirection dir <*> readMaybe n -- read as "'Command (readDirection dir) (readMaybe n)' but with something funky going on"; in this case "something funky" is "maybe failing over to Nothing"
  • I would recommend just taking input from stdin instead of hardcoding a filename. At least on reasonable shells (so not Windows...) you can write something like ./executable < input02 to connect stdin to the file input02.

  • Now we can write main

    -- I like having something like this
    orDie :: String -> Maybe a -> IO a
    orDie msg Nothing = hPutStrLn stderr msg >> exitFailure
    orDie msg (Just x) = return x
    main :: IO ()
    main = do
      input <- orDie "failed to parse" =<< (traverse parseLine <$> lines <$> getContents)
      -- foldl is almost always wrong; it should be either foldr (the idiomatic one) or foldl' (the performant one)
      let finalPos = foldl' (flip move) (Position 0 0) input -- even though we have to flip move, I consider it more idiomatic to have move the way it is and chalk this flip up to "foldl is annoying"
      print $ hPos finalPos * vPos finalPos -- print = putStrLn . show

The collected imports:

import Text.Read(readMaybe)
import System.Exit(exitFailure)
import Data.List(foldl')
import System.IO(hPutStrLn, stderr)
  • \$\begingroup\$ Thanks for taking the time to review it! It didn't even occur to me I could just simply define a function String -> Direction to read the values and parse them instead of tinkering with the Read interface. Also, I must admit I still have to wrap my mind around applicatives (<$> <*>), which is probably why I feel like I'm reinventing the wheel every time I approach a problem. Awesome feedback and explanation, thank you! \$\endgroup\$
    – Jir
    Commented Apr 8, 2022 at 11:42

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.