15
\$\begingroup\$

In the book Real World Haskell, the author writes in the first line of every file that uses, for example the following: file: ch05/PrettyJSON. The chapter slash and the name of a module.

I wanted to create a script which reads the first line and copies the files to the appropriate directory. In the above example we should copy the file to the directory ch05. I wanted to write this in Haskell in order to use it for "real world" applications.

But before starting writing to Haskell I wrote it in Python:

import os
import shutil

current_dir = os.getcwd()
current_files=os.listdir(current_dir)
haskell_files=filter(lambda x:x.find(".hs")!=-1,current_files)

for x in haskell_files:
 with open(x,"r") as f:
     header=f.readline()
    try:
        chIndex=header.index("ch")
    except ValueError as e:
        continue

    number=header[chIndex:chIndex+4]
 try:
    os.mkdir(number)
    shutil.copy2(x,number)
except OSError as e:
    shutil.copy2(x,number)

And then I tried to write the same code in Haskell:

f x = dropWhile (\x-> x/='c') x
g x = takeWhile (\x-> x=='c' || x=='h' || isDigit x) x
z= do 
   y<-getCurrentDirectory
   e<-getDirectoryContents y
   let b=findhs e
   mapM w b
findhs filepaths = filter (isSuffixOf ".hs") filepaths
w filename= do
           y<-readFile filename
           let a=(g.f) y
           if "ch" `isPrefixOf` a
           then do
                 createDirectoryIfMissing False a
                 g<-getCurrentDirectory
                 writeFile (g ++"/"++ a ++ "/" ++ filename) y
            else
                return ()
main=z

Haskell code should be more elegant and more compact but it is not. Can you give me suggestions on making the above program more Haskellish?

\$\endgroup\$

migrated from stackoverflow.com Sep 29 '12 at 19:09

This question came from our site for professional and enthusiast programmers.

17
\$\begingroup\$

Notes:

  • don't use single letter variable names for top level bindings
  • leave spaces around syntax like = and <-
  • use meaningful variable names for system values (such as file contents)
  • inline single uses of tiny functions, such as your findhs e
  • give your core algorithm, w, a meaningful name
  • don't use long names for local parameters like filepaths. fs is fine.
  • use pointfree style for arguments in the final position
  • use when instead of if .. then ... else return ()
  • use the filepath library for nice file path construction
  • your only use of f and g is to compose them. so do that and name the result.
  • use section syntax for lambdas as function arguments to HOFs
  • indent consistently
  • use mapM_ when the result you don't care about.

Resulting in:

import Data.Char
import Data.List
import Control.Monad
import System.Directory
import System.FilePath

clean = takeWhile (\x -> x == 'c' || x == 'h' || isDigit x)
      . dropWhile (/='c')

process path = do
   y <- readFile path

   let a = clean y

   when ("ch" `isPrefixOf` a) $ do
         createDirectoryIfMissing False a
         g <- getCurrentDirectory
         writeFile (g </> a </> path) y

main = do
    y <- getCurrentDirectory
    e <- getDirectoryContents y
    mapM_ process $ filter (isSuffixOf ".hs") e
\$\endgroup\$
  • \$\begingroup\$ much nicer. getCurrentDirectory is superfluous though - in the when clause don't need g, and in main, you can replace y with ".". \$\endgroup\$ – AndrewC Sep 29 '12 at 15:00
  • 1
    \$\begingroup\$ In the sense of the original python code, the name of target directory has to be computed as take 4 <$> find (isPrefixOf "ch") (tails y) where y is extracted from head . lines <$> readFile path . \$\endgroup\$ – David Unric Sep 29 '12 at 15:08
  • 1
    \$\begingroup\$ main can be written as main = getCurrentDirectory >>= getDirectoryContents >>= mapM_ process . filter (isSuffixOf ".hs") \$\endgroup\$ – nponeccop Oct 4 '12 at 16:03
  • \$\begingroup\$ @nponeccop Much more monadic! \$\endgroup\$ – recursion.ninja Jul 13 '15 at 18:24
3
\$\begingroup\$

The first thing you note is that although haskellers like one character variable names in a small scope, we do not like one character functions in a global scope.

There is an implicit partial function in your code. The function that gets the chapter name from the file (called getChapter in the code below), which takes strings (the content of the file) and gives a chapter name if it exists in the string. This seems like a good idea to make explicit using the Maybe type so that this fact does not need to be expressed in the control flow of the program. Note that I have split the getting of the chapter from the contents into the pure part (looking at the string) and the impure part (reading the file), this might be overkill but it's nice to remove impure functions whenever possible.

type Chapter = String

getChapter :: String -> Maybe Chapter
getChapter ('c':'h':xs) = Just ('c':'h': takewhile isDigit xs)
getChapter (_:xs) = getChapter xs
getChapter [] = Nothing

main :: IO ()
main = do 
  dir <- getCurrentDirectory
  files <- getDirectoryContents dir
  mapM_ process (filter (isPrefixOf ".hs") files)

process :: FilePath -> IO ()
process fileName = do
  chapter <- liftM getChapter (readFile fileName)
  when (isJust chapter)
    (copyToChapter fileName (fromJust chapter))

copyToChapter :: FilePath -> Chapter -> IO ()
copyToChapter fileName chapter = do
  createDirectoryIfMissing False chapter
  copyFile fileName (chapter ++ "/" ++ fileName)
\$\endgroup\$
  • \$\begingroup\$ Your getChapter is slightly broken, as it takes only the first digit of the chapter. Also, you waste a lot of time reading the whole file, if the chapter is not specified. \$\endgroup\$ – Karolis Juodelė Sep 30 '12 at 11:56
  • \$\begingroup\$ @KarolisJuodelė I fixed the single digit thing. About wasting a lot of time: The original solution does that too (dropWhile (/='c')) so I just thought it was part of the specification. \$\endgroup\$ – HaskellElephant Sep 30 '12 at 19:12
1
\$\begingroup\$
getChapter :: String -> IO (Maybe String)
getChapter name = if ".hs" `isSuffixOf` name then do
                     ln <- withFile name ReadMode hGetLine
                     return $ fmap (take 4) $ find ("ch" `isPrefixOf`) $ tails ln
                     -- note, this is what your Pythod code does
                  else return Nothing

copyToChapter :: String -> String -> IO ()
copyToChapter file chapter = do
   createDirectoryIfMissing False chapter
   copyFile file (chapter ++ "/" ++ file)

process :: String -> IO ()
process file = do
   chapter <- getChapter file
   maybe (return ()) (copyToChapter file) chapter

main = do 
   dir <- getCurrentDirectory
   files <- getDirectoryContents dir
   mapM process files 

This should be an improvement. Note that this is actually longer. Of course, you could write

getChapter name = if ".hs" `isSuffixOf` name then withFile name ReadMode hGetLine >>= return . fmap (take 4) . find ("ch" `isPrefixOf`) . tails
                  else return Nothing
copyToChapter file chapter = createDirectoryIfMissing False chapter >> copyFile file (chapter ++ "/" ++ file)
process file = getChapter file >>= maybe (return ()) (copyToChapter file)
main = getCurrentDirectory >>= getDirectoryContents >>= mapM process

if you like few lines of code. I'd advise against it though.

In general, you should not expect Haskell to be better at procedural tasks than procedural languages.

\$\endgroup\$
  • 3
    \$\begingroup\$ hould not expect Haskell to be better at procedural tasks than procedural languages. -- but note that because of monads, structuring procedural programs is a lot easier as code blocks are first class. error handling may also be simplified thanks to monads. Abstraction capaabilities always help. \$\endgroup\$ – Don Stewart Sep 29 '12 at 14:18
1
\$\begingroup\$

Other way you could write your Shell-like scripts in Haskell is using Shelly library. It provides functionality specifically suited for such tasks, kind of DSL, which brings you more brevity. Its GitHub page contains a bunch of usefull links with examples.

E.g. Don's program could be rewritten to:

{-# LANGUAGE OverloadedStrings  #-}
import Data.Text.Lazy as LT
import Data.Text.Lazy.IO as LTIO
import Prelude as P hiding (FilePath)
import Data.Char
import Shelly

clean = LT.takeWhile (\x -> x == 'c' || x == 'h' || isDigit x)
      . LT.dropWhile (/='c')

readFileP = liftIO . LTIO.readFile . unpack . toTextIgnore

process path = shelly $ do
  y <- readFileP path
  let a = clean y

  when ("ch" `LT.isPrefixOf` a) $ do
    let p = fromText a
    mkdir_p p
    cp path p

main = shelly $ do
  e <- ls "."
  mapM_ process $ P.filter (hasExt "hs") e

The only flaw here is a bit bloated readFileP. I did not find a better way (which might be due to my beginner level). Possibly such function could become a part of Shelly library itself.

So far I found combination of Haskell and Shelly quite good as "a scripting language for every day task" and start to rewrite my scripts utilizing Shelly. That is a nice entrance gateway to Haskell language itself for me.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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