# Safe Reverse Polish Notation

I implemented the following (what I believe to be) safe Reverse Polish Calculator.

Now I'm using foldM to take advantage of the power of Monads. Previously, I was not.

Can you please critique my code?

import Data.Maybe
import Text.Regex
import Network.CGI.Protocol

data Operator = Add | Subtract | Multiply

-- safe Reverse Polish Notation calculator
rpn :: String -> Maybe Double
rpn xs = maybeStringToMaybeDouble result :: Maybe Double
where result = safeHead $foldM foldingOp []$ words xs

--foldM :: Monad m => (a -> b -> m a) -> a -> [b] -> m a

maybeStringToMaybeDouble :: Maybe String -> Maybe Double
maybeStringToMaybeDouble x = do
y <- x

-- extract maybeHead of Maybe [a]
safeHead :: Maybe [a] -> Maybe a
x <- m -- x has type [a]
listToMaybe x

foldingOp :: [String] -> String -> Maybe [String]
foldingOp acc s
| isNumber s        = Just (s : acc)
| isPlusRegex s     = performOp Add acc
| isMinusRegex s    = performOp Subtract acc
| isMultiplyRegex s = performOp Multiply acc
| otherwise         = Nothing

performOp :: Operator -> [String] -> Maybe [String]
performOp o (x:y:zs) = Just $(doOp o x y) : zs performOp _ _ = Nothing doOp :: Operator -> String -> String -> String doOp Add a b = show$ (read b :: Double) + (read a :: Double)
doOp Subtract a b  = show $(read b :: Double) - (read a :: Double) doOp Multiply a b = show$ (read b :: Double) * (read a :: Double)

isNumber :: String -> Bool
isNumber = matches' mkNumberRegex

mkNumberRegex :: Regex
mkNumberRegex = mkRegex "^[0-9]+[.]?[0-9]*$" isPlusRegex :: String -> Bool isPlusRegex = matches' mkPlusRegex isMinusRegex :: String -> Bool isMinusRegex = matches' mkMinusRegex isMultiplyRegex :: String -> Bool isMultiplyRegex = matches' mkMultiplyRegex mkPlusRegex :: Regex mkPlusRegex = mkRegex "^[+]$"

mkMinusRegex :: Regex
mkMinusRegex = mkRegex "^[-]$" mkMultiplyRegex :: Regex mkMultiplyRegex = mkRegex "^[*]$"

matches' :: Regex -> String -> Bool
matches' r x
| matchRegex r x == Nothing = False
| otherwise                 = True


This:

maybeStringToMaybeDouble x = do
y <- x


Could be:

maybeStringToMaybeDouble x = x >>= maybeRead


Similarly,

safeHead m = do
x <- m
listToMaybe x


Could be

safeHead m = m >>= listToMaybe


Or we could remove these functions entirely and just write

rpn xs = foldM foldingOp [] (words xs) >>= listToMaybe >>= maybeRead


It seems strange that we're importing Network.CGI.Protocol for maybeRead. I would suggest using readMaybe from Text.Read instead.

There's a chance to use isJust here:

matches' :: Regex -> String -> Bool
matches' r x
| matchRegex r x == Nothing = False
| otherwise                 = True

matches' :: Regex -> String -> Bool
matches' r x = isJust $matchRegex r x  We can do away with six functions by rewriting foldingOp like this: foldingOp :: [String] -> String -> Maybe [String] foldingOp acc s | isNumber s = Just (s : acc) | s == "+" = performOp Add acc | s == "-" = performOp Subtract acc | s == "*" = performOp Multiply acc | otherwise = Nothing  And we can simplify further by removing the Operator type: foldingOp :: [String] -> String -> Maybe [String] foldingOp acc s | isNumber s = Just (s : acc) | s == "+" = performOp (+) acc | s == "-" = performOp (-) acc | s == "*" = performOp (*) acc | otherwise = Nothing performOp :: (Double -> Double -> Double) -> [String] -> Maybe [String] performOp o (x:y:zs) = Just$ doOp o x y : zs
performOp _ _        = Nothing

doOp :: (Double -> Double -> Double) -> String -> String -> String
doOp op a b = show \$ (read b :: Double) op (read a :: Double)