Skip to main content
Code Review

Return to Answer

added 880 characters in body
Source Link
Gurkenglas
  • 3.8k
  • 13
  • 17

Inline things used only once,The general idea is to use library combinatorscode, particularly to eliminate explicit recursion, and inline things used only once.

When you implement a transformation of a simple partial algorithm into a safe one, chances are safe's already got something, here splitAtExactMay.

(,,) in its prefix form allows me to not have to close each option with a multi-line closing bracket, and shuddup indentation blocks split code into units just as well as names do, and then you don't need to choose names for everything. Each option even already lists a description of what it does!

import System.Exit (exitSuccess)
import Control.Monad.State
import Control.Monad.Trans.Maybe
import Safe.Exact (splitAtExactMay)
import Safe (fromJustNote)
type TodoList = [String]
main :: IO ()
main = do
 putStrLn "TODO-LIST\n"
 (`evalStateT` []) $ forever $ do
 say "----------------------------"
 say "You have to do these things:"
 say ∘ unlines ∘ zipWith (\i e -> show i ++ ": " ++ show e) ([0..] :: [Integer]) =<< get
 input <- desperately $ do
 say "Select an action"
 say ∘ unlines $ map (\(char, desc, _) -> char : ") " ++ desc) options
 [c] <- hear
 return c
 case find (\(c, _, _) -> c == input) options of
 Just (_, _, action) -> action
 _ -> say "action not found"
desperately :: Monad m => MaybeT m a -> m a
desperately = fmap (fromJustNote "desperately") . runMaybeT . asum . repeat

say = liftIO . putStrLn
hear = liftIO getLine

-- to To bring those previousmain's last lines closer in method with the rest of your code, you could do Just (_, _, action) <- find (\(c, _, _) -> c == input) options and put another desperately to the right of forever. (Or runMaybeT, because the forever repeats everything anyway, but that's kinda incidental.)

When you implement a transformation of a simple partial algorithm into a safe one, chances are safe's already got something, here splitAtExactMay.

(,,) in its prefix form allows me to not close each option with a multi-line closing bracket, and shuddup indentation blocks split code into units just as well as names do, and then you don't need to choose names for everything. Each option even already lists a description of what it does!

import Safe.Exact (splitAtExactMay) -- goes at the top of the file, but relevant here
options :: [(Char, String, StateT TodoList IO ())]
options =
 [ (,,) 'c' "create new item" $ do
 say "enter a description of the new item:"
 hear >>= modify . (:)
 , (,,) 'd' "delete item" $ desperately $ do
 say "enter the number of the item you want to delete"
 Just n <- readMaybe <$> hear
 Just (before, _ : after) <- gets $ splitAtExactMay n
 put $ before ++ after
 , (,,) 'x' "delete everything" $ put []
 , (,,) 'l' "load file" $ desperately $ do
 say "enter the file you want to load"
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line ReadMode
 liftIO $ hSetEncoding file utf8
 Right content <- liftIO $ tryIOError $ hGetContents file
 modify (++ lines content)
 , (,,) 's' "save to file" $ desperately $ do
 say "enter the filename to save your todolist. The file will be overwritten."
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line WriteMode
 liftIO $ hSetEncoding file utf8
 result <- liftIO . tryIOError . hPutStr file . unlines =<< get
 case result of
 Left _ -> say "couldn't write to file"
 Right _ -> liftIO $ hClose file
 , (,,) 'q' "quit" exitSuccess
 ]
desperately :: Monad m => MaybeT m a -> m a
desperately = fmap (fromJustNote "desperately") . runMaybeT . asum . repeat
say = liftIO . putStrLn
hear = liftIO getLine

Inline things used only once, use library combinators, particularly to eliminate explicit recursion.

When you implement a transformation of a simple partial algorithm into a safe one, chances are safe's already got something, here splitAtExactMay.

(,,) in its prefix form allows me to not have to close each option with a multi-line closing bracket, and shuddup indentation blocks split code into units just as well as names do, and then you don't need to choose names for everything. Each option even already lists a description of what it does!

import System.Exit (exitSuccess)
import Control.Monad.State
import Control.Monad.Trans.Maybe
import Safe.Exact (splitAtExactMay)
import Safe (fromJustNote)
type TodoList = [String]
main :: IO ()
main = do
 putStrLn "TODO-LIST\n"
 (`evalStateT` []) $ forever $ do
 say "----------------------------"
 say "You have to do these things:"
 say ∘ unlines ∘ zipWith (\i e -> show i ++ ": " ++ show e) ([0..] :: [Integer]) =<< get
 input <- desperately $ do
 say "Select an action"
 say ∘ unlines $ map (\(char, desc, _) -> char : ") " ++ desc) options
 [c] <- hear
 return c
 case find (\(c, _, _) -> c == input) options of
 Just (_, _, action) -> action
 _ -> say "action not found"

-- to bring those previous lines closer in method with the rest of your code, you could do Just (_, _, action) <- find (\(c, _, _) -> c == input) options and put another desperately to the right of forever. (Or runMaybeT, because the forever repeats everything anyway, but that's kinda incidental.)

options :: [(Char, String, StateT TodoList IO ())]
options =
 [ (,,) 'c' "create new item" $ do
 say "enter a description of the new item:"
 hear >>= modify . (:)
 , (,,) 'd' "delete item" $ desperately $ do
 say "enter the number of the item you want to delete"
 Just n <- readMaybe <$> hear
 Just (before, _ : after) <- gets $ splitAtExactMay n
 put $ before ++ after
 , (,,) 'x' "delete everything" $ put []
 , (,,) 'l' "load file" $ desperately $ do
 say "enter the file you want to load"
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line ReadMode
 liftIO $ hSetEncoding file utf8
 Right content <- liftIO $ tryIOError $ hGetContents file
 modify (++ lines content)
 , (,,) 's' "save to file" $ desperately $ do
 say "enter the filename to save your todolist. The file will be overwritten."
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line WriteMode
 liftIO $ hSetEncoding file utf8
 result <- liftIO . tryIOError . hPutStr file . unlines =<< get
 case result of
 Left _ -> say "couldn't write to file"
 Right _ -> liftIO $ hClose file
 , (,,) 'q' "quit" exitSuccess
 ]
desperately :: Monad m => MaybeT m a -> m a
desperately = fmap (fromJustNote "desperately") . runMaybeT . asum . repeat
say = liftIO . putStrLn
hear = liftIO getLine

The general idea is to use library code, particularly to eliminate explicit recursion, and inline things used only once.

import System.Exit (exitSuccess)
import Control.Monad.State
import Control.Monad.Trans.Maybe
import Safe (fromJustNote)
type TodoList = [String]
main :: IO ()
main = do
 putStrLn "TODO-LIST\n"
 (`evalStateT` []) $ forever $ do
 say "----------------------------"
 say "You have to do these things:"
 say ∘ unlines ∘ zipWith (\i e -> show i ++ ": " ++ show e) ([0..] :: [Integer]) =<< get
 input <- desperately $ do
 say "Select an action"
 say ∘ unlines $ map (\(char, desc, _) -> char : ") " ++ desc) options
 [c] <- hear
 return c
 case find (\(c, _, _) -> c == input) options of
 Just (_, _, action) -> action
 _ -> say "action not found"
desperately :: Monad m => MaybeT m a -> m a
desperately = fmap (fromJustNote "desperately") . runMaybeT . asum . repeat

say = liftIO . putStrLn
hear = liftIO getLine

To bring main's last lines closer in method with the rest of your code, you could do Just (_, _, action) <- find (\(c, _, _) -> c == input) options and put another desperately to the right of forever. (Or runMaybeT, because the forever repeats everything anyway, but that's kinda incidental.)

When you implement a transformation of a simple partial algorithm into a safe one, chances are safe's already got something, here splitAtExactMay.

(,,) in its prefix form allows me to not close each option with a multi-line closing bracket, and shuddup indentation blocks split code into units just as well as names do, and then you don't need to choose names for everything. Each option even already lists a description of what it does!

import Safe.Exact (splitAtExactMay) -- goes at the top of the file, but relevant here
options :: [(Char, String, StateT TodoList IO ())]
options =
 [ (,,) 'c' "create new item" $ do
 say "enter a description of the new item:"
 hear >>= modify . (:)
 , (,,) 'd' "delete item" $ desperately $ do
 say "enter the number of the item you want to delete"
 Just n <- readMaybe <$> hear
 Just (before, _ : after) <- gets $ splitAtExactMay n
 put $ before ++ after
 , (,,) 'x' "delete everything" $ put []
 , (,,) 'l' "load file" $ desperately $ do
 say "enter the file you want to load"
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line ReadMode
 liftIO $ hSetEncoding file utf8
 Right content <- liftIO $ tryIOError $ hGetContents file
 modify (++ lines content)
 , (,,) 's' "save to file" $ desperately $ do
 say "enter the filename to save your todolist. The file will be overwritten."
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line WriteMode
 liftIO $ hSetEncoding file utf8
 result <- liftIO . tryIOError . hPutStr file . unlines =<< get
 case result of
 Left _ -> say "couldn't write to file"
 Right _ -> liftIO $ hClose file
 , (,,) 'q' "quit" exitSuccess
 ]
added 880 characters in body
Source Link
Gurkenglas
  • 3.8k
  • 13
  • 17

forever defeats the need for loop to manually loop.

Contrary to that comment, abstraction is kinda Haskell's thing.

import System.Exit (exitSuccess)
import Control.Monad.State
import Control.Monad.Trans.Maybe
import Safe.Exact (splitAtExactMay)
import Safe (fromJustNote)
type TodoList = [String]
main :: IO ()
main = do
 putStrLn "TODO-LIST\n"
 (`evalStateT` []) $ forever $ do
 say "----------------------------"
 say "You have to do these things:"
 say ∘ unlines ∘ zipWith (\i e -> show i ++ ": " ++ show e) ([0..] :: [Integer]) =<< get
 input <- desperately $ do
 say "Select an action"
 say ∘ unlines $ map (\(char, desc, _) -> char : ") " ++ desc) options
 [c] <- hear
 return c
 case find (\(c, _, _) -> c == input) options of
 Just (_, _, action) -> action
 _ -> say "action not found"

-- to bring those previous lines closer in method with the rest of your code, you could do Just (_, _, action) <- find (\(c, _, _) -> c == input) options and put another desperately to the right of forever. (Or runMaybeT, because the forever repeats everything anyway, but that's kinda incidental.)

options :: [(Char, String, StateT TodoList IO ())]
options =
 [ (,,) 'c' "create new item" $ do
 say "enter a description of the new item:"
 hear >>= modify . (:)
 , (,,) 'd' "delete item" $ desperately $ do
 say "enter the number of the item you want to delete"
 Just n <- readMaybe <$> hear
 Just (before, _ : after) <- gets $ splitAtExactMay n
 put $ before ++ after
 , (,,) 'x' "delete everything" $ put []
 , (,,) 'l' "load file" $ desperately $ do
 say "enter the file you want to load"
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line ReadMode
 liftIO $ hSetEncoding file utf8
 Right content <- liftIO $ tryIOError $ hGetContents file
 modify (++ lines content)
 , (,,) 's' "save to file" $ desperately $ do
 say "enter the filename to save your todolist. The file will be overwritten."
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line WriteMode
 liftIO $ hSetEncoding file utf8
 result <- liftIO . tryIOError . hPutStr file . unlines =<< get
 case result of
 Left _ -> say "couldn't write to file"
 Right _ -> liftIO $ hClose file
 , (,,) 'q' "quit" exitSuccess
 ]
desperately :: Monad m => MaybeT m a -> m a
desperately = fmap (fromJustNote "desperately") . runMaybeT . asum . repeat
say = liftIO . putStrLn
hear = liftIO getLine

Contrary to that comment, abstraction is kinda Haskell's thing.

import System.Exit (exitSuccess)
import Control.Monad.State
import Control.Monad.Trans.Maybe
import Safe.Exact (splitAtExactMay)
import Safe (fromJustNote)
type TodoList = [String]
main :: IO ()
main = do
 putStrLn "TODO-LIST\n"
 (`evalStateT` []) $ forever $ do
 say "----------------------------"
 say "You have to do these things:"
 say ∘ unlines ∘ zipWith (\i e -> show i ++ ": " ++ show e) ([0..] :: [Integer]) =<< get
 input <- desperately $ do
 say "Select an action"
 say ∘ unlines $ map (\(char, desc, _) -> char : ") " ++ desc) options
 [c] <- hear
 return c
 case find (\(c, _, _) -> c == input) options of
 Just (_, _, action) -> action
 _ -> say "action not found"
options :: [(Char, String, StateT TodoList IO ())]
options =
 [ (,,) 'c' "create new item" $ do
 say "enter a description of the new item:"
 hear >>= modify . (:)
 , (,,) 'd' "delete item" $ desperately $ do
 say "enter the number of the item you want to delete"
 Just n <- readMaybe <$> hear
 Just (before, _ : after) <- gets $ splitAtExactMay n
 put $ before ++ after
 , (,,) 'x' "delete everything" $ put []
 , (,,) 'l' "load file" $ desperately $ do
 say "enter the file you want to load"
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line ReadMode
 liftIO $ hSetEncoding file utf8
 Right content <- liftIO $ tryIOError $ hGetContents file
 modify (++ lines content)
 , (,,) 's' "save to file" $ desperately $ do
 say "enter the filename to save your todolist. The file will be overwritten."
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line WriteMode
 liftIO $ hSetEncoding file utf8
 result <- liftIO . tryIOError . hPutStr file . unlines =<< get
 case result of
 Left _ -> say "couldn't write to file"
 Right _ -> liftIO $ hClose file
 , (,,) 'q' "quit" exitSuccess
 ]
desperately :: Monad m => MaybeT m a -> m a
desperately = fmap (fromJustNote "desperately") . runMaybeT . asum . repeat
say = liftIO . putStrLn
hear = liftIO getLine

forever defeats the need for loop to manually loop.

Contrary to that comment, abstraction is kinda Haskell's thing.

import System.Exit (exitSuccess)
import Control.Monad.State
import Control.Monad.Trans.Maybe
import Safe.Exact (splitAtExactMay)
import Safe (fromJustNote)
type TodoList = [String]
main :: IO ()
main = do
 putStrLn "TODO-LIST\n"
 (`evalStateT` []) $ forever $ do
 say "----------------------------"
 say "You have to do these things:"
 say ∘ unlines ∘ zipWith (\i e -> show i ++ ": " ++ show e) ([0..] :: [Integer]) =<< get
 input <- desperately $ do
 say "Select an action"
 say ∘ unlines $ map (\(char, desc, _) -> char : ") " ++ desc) options
 [c] <- hear
 return c
 case find (\(c, _, _) -> c == input) options of
 Just (_, _, action) -> action
 _ -> say "action not found"

-- to bring those previous lines closer in method with the rest of your code, you could do Just (_, _, action) <- find (\(c, _, _) -> c == input) options and put another desperately to the right of forever. (Or runMaybeT, because the forever repeats everything anyway, but that's kinda incidental.)

options :: [(Char, String, StateT TodoList IO ())]
options =
 [ (,,) 'c' "create new item" $ do
 say "enter a description of the new item:"
 hear >>= modify . (:)
 , (,,) 'd' "delete item" $ desperately $ do
 say "enter the number of the item you want to delete"
 Just n <- readMaybe <$> hear
 Just (before, _ : after) <- gets $ splitAtExactMay n
 put $ before ++ after
 , (,,) 'x' "delete everything" $ put []
 , (,,) 'l' "load file" $ desperately $ do
 say "enter the file you want to load"
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line ReadMode
 liftIO $ hSetEncoding file utf8
 Right content <- liftIO $ tryIOError $ hGetContents file
 modify (++ lines content)
 , (,,) 's' "save to file" $ desperately $ do
 say "enter the filename to save your todolist. The file will be overwritten."
 line <- hear
 Right file <- liftIO $ tryIOError $ openFile line WriteMode
 liftIO $ hSetEncoding file utf8
 result <- liftIO . tryIOError . hPutStr file . unlines =<< get
 case result of
 Left _ -> say "couldn't write to file"
 Right _ -> liftIO $ hClose file
 , (,,) 'q' "quit" exitSuccess
 ]
desperately :: Monad m => MaybeT m a -> m a
desperately = fmap (fromJustNote "desperately") . runMaybeT . asum . repeat
say = liftIO . putStrLn
hear = liftIO getLine
added 880 characters in body
Source Link
Gurkenglas
  • 3.8k
  • 13
  • 17

MaybeT abstracts computations that can fail and abort at some point, and allows you to bind into pattern matches that fail the computation if they don't match.

desperately abstracts retrying them until they work, using MaybeTs Alternative instance.

StateT abstracts computations that carry around a piece of state to read and write to.

Contrary to that comment, abstraction is kinda Haskell's thing.

When you implement a transformation of a simple partial algorithm into a safe one, chances are safe's already got something, here splitAtExactMay.

(,,) in its prefix form allows me to not have to close each option with a multi-line closing bracket, and shuddup indentation blocks split code into units just as well as names do, and then you don't need to choose names for everything. Each option even already lists a description of what it does!

MaybeT abstracts computations that can fail and abort at some point, and allows you to bind into pattern matches that fail the computation if they don't match.

desperately abstracts retrying them until they work, using MaybeTs Alternative instance.

StateT abstracts computations that carry around a piece of state to read and write to.

Contrary to that comment, abstraction is kinda Haskell's thing.

When you implement a transformation of a simple partial algorithm into a safe one, chances are safe's already got something, here splitAtExactMay.

(,,) in its prefix form allows me to not have to close each option with a multi-line closing bracket, and shuddup indentation blocks split code into units just as well as names do, and then you don't need to choose names for everything. Each option even already lists a description of what it does!

[Edit removed during grace period]
Source Link
Gurkenglas
  • 3.8k
  • 13
  • 17
Loading
deleted 22 characters in body
Source Link
Gurkenglas
  • 3.8k
  • 13
  • 17
Loading
deleted 32 characters in body
Source Link
Gurkenglas
  • 3.8k
  • 13
  • 17
Loading
Source Link
Gurkenglas
  • 3.8k
  • 13
  • 17
Loading
lang-hs

AltStyle によって変換されたページ (->オリジナル) /