CODEWITHSUNDEEP
haskell
jazmit
jazmit

Reputation: 5410

How can duplication be eliminated from the following Haskell snippet?

The following function clearly has duplication between the two list comprehensions, but how can it be eliminated without increasing the total length of the code? I've got a sneaky feeling there's a nice abstraction lurking here but I just can't see it...

letterAt :: [Word] -> Int -> Int -> Maybe Char 
letterAt wrds x y = listToMaybe $ 
  [wtext !! (x - wx) | 
    Word wx wy wtext Across <- wrds, 
    wy == y && x >= wx && x < wx + length wtext] ++ 
  [wtext !! (y - wy) | 
    Word wx wy wtext Down <- wrds, 
    wx == x && y >= wy && y < wy + length wtext]

Some background:

The function is taken from a crossword program. The crossword is represented as [Word], where

data Word = Word { startX :: Int, 
                   startY :: Int, 
                   text :: String, 
                   sense :: Sense } 

data Sense = Across | Down

The words where sense == Across start at position (startX, startY) and continue in the positive x direction, and those where sense == Down continue in the positive y direction. The aim of the function is to get the character at location (x, y) in a Just, or Nothing if there isn't a character there.

Feel free to point out any other sins against Haskell I've committed here, I've just started with the language and still trying to get to grips with it!

Upvotes: 2

Views: 175

Answers (2)

jazmit
jazmit

Reputation: 5410

Satvik, thanks for your answer which got me thinking along the right lines. I separated out the condition and transformation functions as you suggested, then realized that it would be simpler to transform the data rather than the functions and put everything back into a list comprehension for readability:

letterAt :: [Word] -> Int -> Int -> Maybe Char 
letterAt wrds x y = listToMaybe 
  [wtext !! x' | Word wx wy wtext sens <- wrds,   
                 let (x', y') = case sens of 
                       Across -> (x - wx, y - wy) 
                       Down   -> (y - wy, x - wx), 
                 y' == 0, x' >= 0, x' < length wtext ]

You pointed out that it's better to use Data.find in this case.. is this for readability or efficiency? I'm guessing that because lists are lazy in Haskell, the above code would stop after the first item in the list comprehension was evaluated, is this right?

Upvotes: 1

Satvik
Satvik

Reputation: 11208

Here are some points about your code:

  • It is better to use filter when want to select certain elements of a list based on the predicate.
  • Since you just want the first element satisfying certain predicate you can use Data.List.find

Your conditions looks symmetric so you can define a transform function as 

transform f x y (Word wx wy wtext Across) = f wtext wy wx y x
transform f x y (Word wx wy wtext Down) = f wtext wx wy x y

Now writing the code will require writing conditions only once 

letterAt :: [Word] -> Int -> Int -> Maybe Char
letterAt wrds x y = (transform charValue x y) <$> find (transform condition x y)  wrds
    where
        condition wtext wx wy x y = wx == x && y >= wy && y < wy + length wtext
        charValue wtext wx wy x y = wtext !! (y-wy)

Upvotes: 4

Related Questions