Question

I was writing some Haskell this afternoon and I have a list of conditions that must be satisfied. If they are all true I want to return true, if one of them is false then return false.

I have a method that is working but I was just wondering if there is a better way to implement it for readability/efficiency.

Here is what I have:

checkMatch :: Person -> Person -> Bool
checkMatch seeker candidate
    |  gender candidate == preferedGender seeker 
    && gender seeker == preferedGender candidate
    && minAcceptableAge seeker <= age candidate 
    && maxAcceptableAge seeker >= age candidate
    && minAcceptableAge candidate <= age seeker
    && maxAcceptableAge candidate >= age seeker = True
    |  otherwise = False

Gender is defined as:

data Gender = Male | Female (Eq)

So I just aligned the &&'s and |'s to make this look a little better but I feel there has to be a better way, but can't seem to come up with anything searching Google.

Était-ce utile?

La solution

You can lose the guards and use and to check your conditions:

checkMatch :: Person -> Person -> Bool
checkMatch seeker candidate = and [ 
    gender candidate           == preferedGender seeker 
  , gender seeker              == preferedGender candidate
  , minAcceptableAge seeker    <= age candidate 
  , maxAcceptableAge seeker    >= age candidate
  , minAcceptableAge candidate <= age seeker
  , maxAcceptableAge candidate >= age seeker
  ]

Autres conseils

You could abuse the maybe monad for syntactic sugar as:

a |==| b = guard $ a == b
a |>=| b = guard $ a >= b
a |<=| b = guard $ a <= b
a |/=| b = guard $ a /= b

checkMatch :: Person -> Person -> Bool
checkMatch seeker candidate = Just () == do
  gender candidate |==| preferedGender seeker
  gender seeker |==| preferedGender candidate
  minAcceptableAge seeker |<=| age candidate 
  maxAcceptableAge seeker |>=| age candidate
  minAcceptableAge candidate |<=| age seeker
  maxAcceptableAge candidate |>=| age seeker

Well, first, you can and guard-conditions simply with ,.

Next, I should refactor the minAccAge <= age && maxAccAge >= age patter into a dedicated function, say

acceptsAge :: Person -> Age -> Bool
judge `acceptsAge` age
   = age >= minAcceptableAge judge && age <= maxAcceptableAge judge

It remains

checkMatch :: Person -> Person -> Bool
checkMatch seeker candidate
  | gender candidate == preferedGender seeker 
  , gender seeker == preferedGender candidate
  , seeker `acceptsAge` age candidate 
  , candidate `acceptsAge` age seeker         = True

I'd leave it at this then, the two preferedGender checks can't be reduced much.

I noted that your code contains some duplication because you check eveything in both directions. I would define a helper function checkAcceptable that only checks one direction and than call that function twice:

checkAcceptable :: Person -> Person -> Bool
checkAcceptable seeker candidate =
  gender candidate == preferedGender seeker &&
  minAcceptableAge seeker <= age candidate &&
  maxAccetableAge seeker >= age candidate

checkMatch :: Person -> Person -> Bool
checkMatch seeker candidate =
  checkAcceptable seeker candidate &&
  checkAcceptable candidate seeker

For code minimization, you could write it like this

inRange x (a,b) = a <= x && x <= b

checkOneWay a b = gender b == preferredGender a
               && age b `inRange` (minAcceptableAge a, maxAcceptableAge a)

checkMatch candidate seeker = checkOneWay candidate seeker
                           && checkOneWay seeker candidate

What you are asking is a question of style, so there is no right or wrong answer. However, here are a couple of suggestions.

Firstly you are writing the equivalent of this pattern:

isTrue value | value == True = True
             | otherwise     = False

Which can of course be simplified to:

isTrue value = value

Secondly, as you are checking multiple tests that all need to be true, you can make use of the and function and pass your tests as elements of a list. This will return True if all elements are True, otherwise it will short-circuit and return False.

Putting these two ideas together we get:

checkMatch :: Person -> Person -> Bool
checkMatch seeker candidate
  = and [gender candidate == preferedGender seeker, 
         gender seeker == preferedGender candidate,
         minAcceptableAge seeker <= age candidate,
         maxAcceptableAge seeker >= age candidate,
         minAcceptableAge candidate <= age seeker,
         maxAcceptableAge candidate >= age seeker]

...which is probably how I would write it.

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top