Home > Software design >  Simplify function for Haskell
Simplify function for Haskell

Time:10-31

I have this code, which in my opinion is too long. And I was wondering if there's a way for me to simplify or shorten it.

I have this helper-function swap which takes in two lists of integers and returns them after swapping a number from on to the other All lists have 5 ints in them, so I fill in with zeroes if there are less than that.

It looks like this:

-- Takes in two lists and moves one non-zero int from one to the other
swap :: [Int] -> [Int] -> [[Int]]
swap xs ys
  | xs == ys = [xs, ys]                                             -- If both lists are all zeroes
  | all (==0) ys =                                                  -- If the second list is all zeroes
    let
        e = head $ filter (/= 0) xs                                 -- First non-zero element from first list
        newX = replicate (length xs - length (filter (/= 0) xs) - 1) 0    tail (filter (/= 0) xs)
        newY = tail ys    [e]
        in
            [newX, newY]
    | otherwise =
    let
        e = head $ filter (/= 0) xs                                 -- First non-zero element from first list
        newX = replicate (length xs - length (filter (/= 0) xs)   1) 0    tail (filter (/= 0) xs)
        newY = replicate (length (filter (==0) ys) - 1) 0    [e]    drop (length xs - length (filter (/= 0) ys)) ys
        in
            [newX, newY]

I am using it in my function below, which is the function I feel like is too long:

move :: Int -> Int -> [[Int]] -> [[Int]]
move a b ints
  | a == b = error "a and b cannot be the same integer"
  | a == 1 && b == 2 =
    let
        fst = ints !! (a-1)
        snd = ints !! (b-1)
        swapped = swap fst snd
        in
            [head swapped, last swapped, last ints]
  | a == 1 && b == 3 =
    let
        fst = ints !! (a-1)
        snd = ints !! (b-1)
        swapped = swap fst snd
        in
            [head swapped, ints !! 1, last swapped]
  | a == 2 && b == 1 =
    let
        fst = ints !! (a-1)
        snd = ints !! (b-1)
        swapped = swap fst snd
        in
            [last swapped, head swapped, last ints]
  | a == 2 && b == 3 =
    let
        fst = ints !! (a-1)
        snd = ints !! (b-1)
        swapped = swap fst snd
        in
            [head ints, head swapped, last swapped]
  | a == 3 && b == 1 =
    let
        fst = ints !! (a-1)
        snd = ints !! (b-1)
        swapped = swap fst snd
        in
            [last swapped, ints !! 1, head swapped]
  | a == 3 && b == 2 =
    let
        fst = ints !! (a-1)
        snd = ints !! (b-1)
        swapped = swap fst snd
        in
            [head ints, last swapped, head swapped]
  | otherwise = error "a and b must be either 1, 2 or 3"

This function takes in two ints and a list consisting of 3 lists of ints, where each list has 5 integers, so a sample input would be f.ex

[[1,2,3,4,5],[0,0,0,0,0],[0,0,0,0,0]] or 
[[0,0,0,4,5],[0,0,0,2,3],[0,0,0,0,1]] or 
[[0,0,3,4,5],[0,0,0,0,1],[0,0,0,0,2]] 

So $a$ and $b$ can only be either 1,2, or 3. Giving me $2^3=6$ different cases to consder. Even If manage to get it down to two cases $a>b$ and $b>a$ I still get too many lines of code compared to what I expect is proper for Haskell. Is there any way of shortening this code?

CodePudding user response:

This is not a full answer yet, but the start of one. Look how I removed all code duplication into where blocks. There are still plenty of opportunities of simplification.

-- Takes in two lists and moves one non-zero int from one to the other
swap :: [Int] -> [Int] -> [[Int]]
swap xs ys
    | xs == ys = [xs, ys]                                             -- If both lists are all zeroes
    | all (==0) ys =                                                  -- If the second list is all zeroes
      let
        newX = replicate (length xs - length nonZeroXs - 1) 0    tail nonZeroXs
        newY = tail ys    [e]
      in
        [newX, newY]
    | otherwise =
      let
        newX = replicate (length xs - length nonZeroXs   1) 0    tail nonZeroXs
        newY = replicate (length (filter (==0) ys) - 1) 0    [e]    drop (length xs - length (filter (/= 0) ys)) ys
      in
        [newX, newY]
  where
    nonZeroXs = filter (/= 0) xs
    e = head $ filter (/= 0) xs                                 -- First non-zero element from first list

The only difference between newXs is the signal of the the final addition. Which suggests that the entire decision block might be not be at it's best place.

move :: Int -> Int -> [[Int]] -> [[Int]]
move a b ints
    | a == b = error "a and b cannot be the same integer"
    | a == 1 && b == 2 = [head swapped, last swapped, last ints]
    | a == 1 && b == 3 = [head swapped, ints !! 1,    last swapped]
    | a == 2 && b == 1 = [last swapped, head swapped, last ints]
    | a == 2 && b == 3 = [head ints,    head swapped, last swapped]
    | a == 3 && b == 1 = [last swapped, ints !! 1,    head swapped]
    | a == 3 && b == 2 = [head ints,    last swapped, head swapped]
    | otherwise = error "a and b must be either 1, 2 or 3"
  where 
    fst = ints !! (a-1)
    snd = ints !! (b-1)
    swapped = swap fst snd

Have a good look on the above and run some tests. I don't have the time to test it myself right now.

  • Related