HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

INI File Parser in Haskell

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
parserfileinihaskell

Problem

I'm learning Haskell at the moment, and have just written my first useful module - a parser fo INI files. I used Parsec. I'd like to know what can be improved here - or maybe I did some things completely wrong and there is a better way. Thanks.

module IniFile (iniFileToMap) where

import Text.ParserCombinators.Parsec
import qualified Data.Map as Map
import Char

parseIniFile :: Parser ([(String, [(String, String)])])
parseSection :: Parser (String, [(String, String)])
parseSectionHeader :: Parser String
parseNameValue :: Parser (String, String)
parseComment :: Parser ()
parseNormalValue :: Parser String
parseQuotedValue :: Parser String
parseValue :: Parser String
parseName :: Parser String
parseInsideQuotes :: Parser String
parseCmtOrSpace :: Parser ()
iniFileToMap :: FilePath -> IO (Map.Map String (Map.Map String String))

parseCmtOrSpace = do{
   spaces;
   skipMany parseComment;
}

parseInsideQuotes = many (noneOf "\"")

parseQuotedValue = between (char '"') (char '"') parseInsideQuotes 

parseNormalValue = many1 (satisfy (\c -> isPrint c && not (isSpace c)))

parseValue = parseQuotedValue  parseNormalValue

parseName =  many1 (satisfy (\c -> isAlpha c || isDigit c || c == '_' || c == '.'));

parseComment = do{
   skipMany1 (char ';');
   skipMany (noneOf "\n");
   spaces;
}

parseNameValue = do{
   name  (fst e, Map.fromList (snd e))) list)

iniFileToMap path = do{
   result  error (show err)
         Right xs -> return (list2Map(xs));
}

Solution

The first thing I noticed is that you have grouped all the type signatures together, away from the function bodies. This seems very strange to me. It's way more common (and as far as I'm concerned also way more readable) to have a value's type directly before its definition.

The second thing I noticed were all the braces and semicolons. Unless you have a specific reason to use them, I'd strongly recommend making use of Haskell's significant indentation instead. Much less visual noise. I'm especially confused by the semicolon at the end of parseName's definition as parseName doesn't use do-notation and you didn't use a semicolon on any other definition which didn't use do.

You sometimes surround variables with parentheses (e.g. in values <- endBy1 (parseNameValue) (parseCmtOrSpace), but not in the line name <- between parseCmtOrSpace ... directly above it). I can't see any system to when you do this and when you don't, so it just seems random and inconsistent. So I'd recommend to get rid of the needless parentheses.

Speaking of parentheses, you might also want to get rid of some more parentheses by sprinkling a couple of $ signs in your code, to give it a more "haskellish" feel.

parseSectionHeader = do{
   name <- between (char '[') (char ']') parseName;
   return name;
}

parseIniFile  = do{
   result <- many1 (parseSection);
   return result;
}


Whenever you have something of the form:

foo <- bar
return foo


You can simply just write bar. I.e.:

parseSectionHeader = between (char '[') (char ']') parseName

parseIniFile  = many1 parseSection


iniFileToMap path = do{
   result  error (show err)
         Right xs -> return (list2Map(xs));
}


Here the indentation is off - case should not be indented further than result <-. It should be:

iniFileToMap path = do
    result  error $ show err
        Right xs -> return $ list2Map xs

Code Snippets

parseSectionHeader = do{
   name <- between (char '[') (char ']') parseName;
   return name;
}

parseIniFile  = do{
   result <- many1 (parseSection);
   return result;
}
foo <- bar
return foo
parseSectionHeader = between (char '[') (char ']') parseName

parseIniFile  = many1 parseSection
iniFileToMap path = do{
   result <- parseFromFile parseIniFile path;
      case (result) of
         Left err -> error (show err)
         Right xs -> return (list2Map(xs));
}
iniFileToMap path = do
    result <- parseFromFile parseIniFile path
    case result of
        Left err -> error $ show err
        Right xs -> return $ list2Map xs

Context

StackExchange Code Review Q#2253, answer score: 6

Revisions (0)

No revisions yet.