haskell binary tree function

I have to write a function in haskell that checks if two binary trees are mirror images of one another. This is what i have so far but i do not know if the True && areMirrorImages l1 r2 && areMirrorImages r1 l2 is the right way to do it. I was trying to logical AND True with the results of recursively calling the areMirrorImages functions.

-- Tests whether two BinaryTrees are mirror images of one another
areMirrorImages :: (Eq (BinaryTree a), Eq a) => BinaryTree a -> BinaryTree a -> Bool
areMirrorImages Empty Empty = True
areMirrorImages _ Empty = False
areMirrorImages Empty _     = False
areMirrorImages (Node x1 left1 right1) (Node x2 left2 right2) = 
    if (x1 == x2 && left1 == right2 && right1 == left2)
then True && (areMirrorImages left1 right2) &开发者_运维问答& (areMirrorImages right1 left2)
else False 

This is the error i get when i try to compile:

A2.hs:2:0:
    Non-type variables, or repeated type variables,
      in the constraint: Eq (BinaryTree a)
    (Use -fglasgow-exts to permit this)
    In the type signature:
      areMirrorImages :: (Eq (BinaryTree a), Eq a) =>
                         BinaryTree a -> BinaryTree a -> Bool

I cant seem to figure out what's wrong

Requiring Eq (BinaryTree a) means that you need to compare (test for Equality) two trees. You don't need to do this because you are testing to see if trees are mirrors of each other, not if they are equal.

When are trees mirrors of each other? When the key is the same and when the opposite branches are mirror images of each other.

The key is the same: x1 == x2

Opposite branches are mirror images: areMirrorImages left1 right2 && areMirrorImages right1 left2

Translate that into Haskell:

areMirrorImages (Node x1 left1 right1) (Node x2 left2 right2) = 
    x1 == x2 && areMirrorImages left1 right2 && areMirrorImages right1 left2

Haskell allows you to write concise, straightforward code. There is no need for using if then else.

Probably the only constraint you need is Eq a, if BinaryTree has a derived instance of Eq. That derived instance will look something like:

instance Eq a => Eq (BinaryTree a) where
    ...

Therefore, knowing that a can be compared for equality will allow Haskell to figure out that your BinaryTree a values can be compared for equality.

Your logic also looks like it might be wrong to me. Your function will return False unless both left1 == right2 and areMirrorImages left1 right2 are True, for example. These cannot both be true unless left1 and right2 are symmetrical, assuming a correct implementation of areMirrorImages.

You've got the right idea. I've trimmed your version down a little bit and it seems to work just fine:

data BinaryTree a = Empty | Node a (BinaryTree a) (BinaryTree a)
                  deriving(Show, Eq)

areMirrorImages :: (Eq a) => BinaryTree a -> BinaryTree a -> Bool
areMirrorImages (Node x1 left1 right1) (Node x2 left2 right2) 
    | x1 == x2  = areMirrorImages left1 right2 &&
                  areMirrorImages right1 left2
    | otherwise = False
areMirrorImages Empty Empty = True
areMirrorImages _ _ = False

So what did I change?

The type signature - In the end, you only really need to invoke == on the elements of the tree, so only a needs to be an instance of Eq. (Even though I did derive Eq in the data declaration)

The condition - Like I said, in the end you only really need to test that the particular elements are equal. It doesn't make sense to check if left1 == right2 && right1 == left2 since they shouldn't be equal, these branches should be mirrors of each other.

Guard instead of an if - I think guards are prettier than ifs.

The pattern matches - It is a little cleaner (imho) to just have the catch-all False at the end.

I've been trying to learn QuickCheck lately so I also pumped out this code to prove that areMirrorImages works.

instance (Arbitrary a) => Arbitrary (BinaryTree a) where
    arbitrary = fromList `fmap` arbitrary

fromList :: [a] -> BinaryTree a
fromList []     = Empty
fromList (x:xs) = Node x (fromList lhalf) (fromList rhalf)
    where (lhalf, rhalf) = splitAt (length xs `div` 2) xs

mirror :: BinaryTree a -> BinaryTree a
mirror Empty = Empty
mirror (Node x l r) = Node x (mirror r) (mirror l)

prop_mirror tree = areMirrorImages tree (mirror tree)

My instance of Arbitrary for BinaryTree isn't the best; it only makes nearly-balanced trees. But I figured it was pretty good. Also note that prop_mirror only tests for when areMirrorImages should return True; it does not expose potential false positives (though I'm fairly sure there will be none).

*Main> quickCheck prop_mirror 
+++ OK, passed 100 tests.