patternMinor
Safe Reverse Polish Notation
Viewed 0 times
reversepolishsafenotation
Problem
I implemented the following (what I believe to be) safe Reverse Polish Calculator.
Now I'm using
Can you please critique my code?
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 Control.Monad
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 Maybe a
safeHead m = do
x 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 = TrueSolution
This:
Could be:
Similarly,
Could be
Or we could remove these functions entirely and just write
It seems strange that we're importing
There's a chance to use
We can do away with six functions by rewriting
And we can simplify further by removing the
maybeStringToMaybeDouble x = do
y <- x
maybeRead yCould be:
maybeStringToMaybeDouble x = x >>= maybeReadSimilarly,
safeHead m = do
x <- m
listToMaybe xCould be
safeHead m = m >>= listToMaybeOr we could remove these functions entirely and just write
rpn xs = foldM foldingOp [] (words xs) >>= listToMaybe >>= maybeReadIt 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 = Truematches' :: Regex -> String -> Bool
matches' r x = isJust $ matchRegex r xWe 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 = NothingAnd 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)Code Snippets
maybeStringToMaybeDouble x = do
y <- x
maybeRead ymaybeStringToMaybeDouble x = x >>= maybeReadsafeHead m = do
x <- m
listToMaybe xsafeHead m = m >>= listToMayberpn xs = foldM foldingOp [] (words xs) >>= listToMaybe >>= maybeReadContext
StackExchange Code Review Q#61137, answer score: 4
Revisions (0)
No revisions yet.