current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite 
guessCompilers :: Options -> IO Options
guessCompilers options =  foldl (>>=) (return options) modifiers where 
  modifiers = 
    [guessMetapost 
     -- guessAsymptote, 
     -- guessTex, 
     -- guessLatex, 
     -- guessXelatex
    ]   
guessMetapost :: Options -> IO Options
guessMetapost opts = if isJust $ optMetaPost opts 
                         then 
                           putStrLn $ printf "using enforced Metapost compiler: %s" (fromJust $optMetaPost opts)
                           >> return opts
                         else do 
                           cc <- compiler Metapost.interface
                           if isJust cc then putStrLn $ printf "found Metapost compiler: %s" (fromJust cc)
                                             else putStrLn "found Metapost compiler: none"

Here is part of my project(build system). Compiler can be spectified by cmd args or can be guessed. But double check for Nothing is not cool in my opinion. Also, I heard fromJust is evil. Any suggestions to improve my code?

share|improve this question

1 Answer 1

up vote 2 down vote accepted

Pattern match instead of using isJust and fromJust. (You could use maybe instead of pattern matching, but I think pattern matching is clearer in this case.)

This eliminates the possibility of forgetting to ensure a value is a Just before calling fromJust on it.

guessMetapost :: Options -> IO ()
guessMetapost opts
   = case optMetaPost opts of
         Just mcc -> putStrLn $ printf "using enforced Metapost compiler: %s" mcc
         Nothing -> do
           cc' <- compiler Metapost.interface
           case cc' of
               Just cc -> putStrLn $ printf "found Metapost compiler: %s" cc
               Nothing -> putStrLn "found Metapost compiler: none"

(Your code wasn't consistently returning any options, so I've stripped that bit out.)

share|improve this answer
2  
You can also get rid of putStrLn $ printf ..., and simply write printf. There is an IO a instance of PrintfType. –  danr Jul 30 '12 at 15:01

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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