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.