CODEWITHSUNDEEP
haskell
Babra Cunningham
Babra Cunningham

Reputation: 2967

Improve overly complex haskell list mapping?

I have two functions that perform the following:

Take two lists of Ints (list1, list2).

Take the average of the first list, and make a range value (r).

Map over list2 to check if each element is within the range, return 0 if outside range.

Convert any list2 elements that are returned as 0 into False and other values to True.

Check is list2 contains any False values.

Code: 

checkList :: Int -> Int -> Int -> Int --Check if element is within range
checkList element ave r
   | element >= (ave + (ave + r)) = 0
   | element <= (ave - (ave + r)) = 0
   | otherwise = e

mainFunc :: [Int] -> [Int] -> (Int -> Int -> Int -> Int) -> Bool
mainFunc list1 list2 checkL = elem (False) resultIntToBool --Check for any False elements
   where
      resultIntToBool = map (\element -> if element == 0 then False else True) result --Convert elements to bools
      result = map (\element -> checkL element ave r) list2 --Pass each element to checkL
      ave = sum list1 `div` length list1 --Calculate average
      r = ave + 30 --Create range value

How can I make this code less verbose and more readable?

Upvotes: 2

Views: 107

Answers (1)

redneb
redneb

Reputation: 23850

I am not sure I understood your problem correctly, but I think what you ask for is to have a function that can check whether all elements of list2 are between avg - r and avg + r where avg is the average of list1 and r is some number.

If that is correct, here's what I would change in your code:

  1. I would move checkList inside mainFunc. This allows you to reference symbols defined in the where clause directly which can make the code more readable in this case.
  2. I would make checkList return a boolean instead of an integer as you were using the integer as if it was a boolean anyway. This would also allow you to greatly simplify the definition of the function, which can now be defined using a single boolean expression (no need for multiple cases).
  3. I would rename checkList to something more descriptive, e.g. checkInRange.
  4. Finally, I would use the function [all] from Prelude (http://hackage.haskell.org/packages/archive/base/latest/doc/html/Prelude.html#v:all) that can check if all elements of a list satisfy a property.

Putting it all together:

mainFunc :: [Int] -> [Int] -> Bool
mainFunc list1 list2 = all checkInRange list2
  where
    checkInRange element = ave - r <= element && element <= ave + r
    ave = sum list1 `div` length list1 --Calculate average
    r = 30 --Create range value

Upvotes: 3

Related Questions