6
\$\begingroup\$

I've implemented a very simple in-memory "database" just as an exercise. I wanted to see if there's anything obvious I should be doing that fits more of the Haskell way of doing things.

import qualified Data.Map as M

type History = [String]

data Result = Result (Maybe String) Bool DB

data Command = Command { name  :: String
                       , key   :: Maybe String
                       , value :: Maybe String
                       }

data DB = DB (M.Map String String)

cmdKey c   = let Just s = key   c in s
cmdValue c = let Just s = value c in s

runSetCmd :: Command -> DB -> IO (Maybe String, DB)
runSetCmd c db@(DB map) = do
    let newMap = M.insert (cmdKey c) (cmdValue c) map
    return $ (Nothing, DB newMap)

runGetCmd :: Command -> DB -> IO (Maybe String, DB)
runGetCmd c db@(DB map) = return $ (M.lookup (cmdKey c) map, db)

execCmd :: DB -> Command -> IO Result
execCmd db c@(Command name key value) = do
    (output,newDB) <- case name of
        "set" -> runSetCmd c db
        "get" -> runGetCmd c db
        _     -> return (Nothing, db)
    return $ Result output end newDB

  where
    end = case name of
              "end" -> True
              _     -> False

getCmd = getLine >>= return . parseCmd

parseCmd :: String -> Command
parseCmd s =
    case words s of
        (name:key:value:_) -> Command name (Just key) (Just value)
        (name:key:[])      -> Command name (Just key) Nothing
        (name:[])          -> Command name Nothing Nothing

displayResult :: Result -> IO Result
displayResult r@(Result (Just s) _ _) = putStrLn s >> return r
displayResult r                       = return r

continue :: Result -> IO ()
continue (Result _ end db) = if end then return () else repl db

repl state = getCmd >>= execCmd state >>= displayResult >>= continue

main = repl (DB M.empty)
\$\endgroup\$

3 Answers 3

5
\$\begingroup\$

First of all, as Karl said, you should minimize the code in IO monad and make sure most of your code is in pure functions

Second your Command type is not properly designed

data Command = Command { name  :: String
                         , key   :: Maybe String
                         , value :: Maybe String
                       }

Having the name as String is not cool. You could have a type something like

data CommandType = Get | Set

...but still your command type has too many invalid combinations which the type system cannot help you with (for example: name="get", key="xyz", value="asdfasdf"). Your type should be designed such that you avoid these combinations and you make the type system work for you.

Given this I would define the Command type like this:

data Command = Invalid | End | Get String | Set String String

if your type is like this then execCmd will become:

exec (Get key) = ....
exec (Set key value) = ...
exec Exit = ...
exec Invalid = ...

If you only address these two issues you will see a lot of simplification in your code. Other things would be:

Using the State monad will help you with the passing of the current db state between the functions operating on it. If you do this then passing the state will not be your concern and your result could be either a tuple or a record:

data Result = Result { output: String ; end:Bool} 

Then your displayResult will look like this:

UPDATE

If you don't want the a command to output anything then the output field in Result could have the type "Maybe String" instead of String. \

data Result = Result { output: Maybe String ; end:Bool} 

Then you will have to choose only the Just outputs before you display the results.

\$\endgroup\$
2
  • \$\begingroup\$ in the case of "Set", how would your proposed displayResult work ? \$\endgroup\$
    – zinking
    Jun 4, 2016 at 6:57
  • \$\begingroup\$ Good point. See the update \$\endgroup\$
    – vidi
    Jun 4, 2016 at 12:54
2
\$\begingroup\$

runSetCmd, runGetCmd, and execCmd don't need to be in the IO monad.

It feels a bit wrong to have runGetCmd returning a "new DB". Of course, it really returns the same DB, but the caller has no guarantee.

cmdKey and cmdValue are partial functions, which is dangerous. What you could do instead is have a unique data constructor in the Command type for each command, which has exactly the parameters that command needs.

I'd recommend enabling all compiler warnings and making warnings into fatal errors. (That's -Wall -Werror.) It will probably catch some little things in here.

\$\endgroup\$
2
\$\begingroup\$

You have two important effects going on:

  • IO, to interact with the user

  • State, to store the map

You can combine these using monad transformers, which let you layer State on top of IO by using StateT. When you do, the program becomes much more concise:

import Control.Monad (forever)
import Control.Monad.Trans.Class (lift)
import Control.Monad.Trans.State
import Data.Map as M

loop :: StateT (Map String String) IO r
loop = forever $ do
    l <- lift getLine
    case (words l) of
        ["get", key]      -> do
            mval <- gets (M.lookup key)
            lift $ putStrLn $ case mval of
                Nothing  -> "Invalid key"
                Just val -> val
        ["set", key, val] -> modify (M.insert key val)
        _                 -> lift $ putStrLn "Invalid command"

repl :: IO r
repl = evalStateT loop M.empty

If you are new to monad transformers, I highly recommend you read Monad Transformers - Step by Step.

\$\endgroup\$
2
  • \$\begingroup\$ I like this one, simple and neat. however the link is a bit dry to follow. I read the text, repo the code. but I barely understand how/when MonadTransformer shall be used. \$\endgroup\$
    – zinking
    Jun 4, 2016 at 17:16
  • \$\begingroup\$ another question is: how will you handle end command here ? \$\endgroup\$
    – zinking
    Jun 4, 2016 at 17:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.