the below code does what it needs to do. However I would like to know if there are better (shorter) ways to express the below while maintaining the explicitness of showing the decision like with the usage of
first a few definitions
type Numbers = Array Number type Sizes = Tuple Int Int foreign import _dot :: Numbers -> Numbers -> Number data Matrix = Matrix (Array Numbers) Sizes data MatrixError = VectorsExpected | InvalidVectorSize Int Int | InvalidRowSize Int | UnexpectedError
then the implementation
dot :: Matrix -> Matrix -> Either MatrixError Number dot (Matrix a sa) (Matrix b sb) = if areSimilarVectors sa sb then Right $ dot' a b else failed sa sb where areSimilarVectors s1 s2 = isVector s1 s2 && isSameSize s1 s2 dot' a b = _dot (join a) (join b) isVector sa sb = fst sa == 1 && fst sb == 1 isSameSize sa sb = snd sa == snd sb failed sa@(Tuple ax ay) sb@(Tuple bx by) = if isVector sa sb then if isSameSize sa sb then Left $ UnexpectedError else Left $ InvalidVectorSize ay by else Left $ VectorsExpected
I experimented with the use of
>>= so my code looked something like (from memory).
First a few utility fns
check' b r = if b then Right r else Left r -- the else case could be dismissed check b r = _ -> if b then Right r else Left r
and here the rewrite
dot (Matrix a sa@(Tuple ax ay)) (Matrix b sb@(Tuple bx by)) = check' (isVector sa sb) VectorsExpected >>= check (isSameSize sa sb) (InvalidVectorSize ay by) >>= check true (dot' a b) where dot' a b = _dot (join a) (join b) isVector sa sb = fst sa == 1 && fst sb == 1 isSameSize sa sb = snd sa == snd sb
This is shorter IF I factor out the
check functions as generic utility functions. But its definitely not clearer (to me at least) what this does.
I am looking forward to your suggestions and some opportunities to learn new stuff.
The code is in Purescript but it should be possible to Haskellers to follow this answer as well.
Without changing the code too drastically I see two immediate improvements, for one you can pull out the
Leftconstructor in front of the call to fail, thus keeping
Left on the same level and making the distinction clearer:
if areSimilarVectors sa sb then Right $ dot' a b else Left $ failed sa sb
Secondly because the code handles failure cases I’d suggest you reverse the predicates for the ifs to reduce the distance between Reason <-> Error
if not $ isVector sa sb then VectorsExpected else if not $ isSameSize sa sb then InvalidVectorSize ay by else UnexpectedError
Other things you might want to consider is the naming of
isVector and the amount of shadowing of
sb. Why are you using a where clause and then still pass the arguments around explicitly? Maybe it would be even better if you matched the
Tuples in the toplevel pattern match and stopped using
snd alltogether? Applying that we get to:
dot :: Matrix -> Matrix -> Either MatrixError Number dot (Matrix a (Tuple ax ay)) (Matrix b (Tuple bx by)) = if areSimilarVectors then Right $ dot' a b else Left failed where dot' a b = _dot (join a) (join b) areSimilarVectors = areVectors && areSameSize areVectors = ax == 1 && bx == 1 areSameSize = ay == by failed = if not areVectors then VectorsExpected else if not areSameSize then InvalidVectorSize ay by else UnexpectedError
I would also look at using guards:
dotProduct :: Num a => [a] -> [a] -> Either String a dotProduct as bs | notSimilar = Left "bad operands" | otherwise = Right $ sum $ zipWith (*) as bs where notSimilar = length as /= length bs
Another option is to use
import Control.Monad import Control.Monad.Except dotProduct'' :: Num a => [a] -> [a] -> Either String a dotProduct'' as bs = do when (length as /= length bs) $ throwError "bad operands" return $ sum $ zipWith (*) as bs
You’re forcing the reader to read the code linearly, but it also removes a level of indentation.
The Either monad allows you to use do notation here:
dot :: Matrix -> Matrix -> Either MatrixError Number dot (Matrix a (Tuple ax ay)) (Matrix b (Tuple bx by)) = do unless (ax == 1 && bx == 1) $ Left VectorsExpected unless (ay == by) $ Left $ InvalidVectorSize ay by Right $ _dot (join a) (join b)