patternMinor
INI File Parser in Haskell
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
You sometimes surround variables with parentheses (e.g. in
Speaking of parentheses, you might also want to get rid of some more parentheses by sprinkling a couple of
Whenever you have something of the form:
You can simply just write
Here the indentation is off -
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 fooYou can simply just write
bar. I.e.:parseSectionHeader = between (char '[') (char ']') parseName
parseIniFile = many1 parseSectioniniFileToMap 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 xsCode Snippets
parseSectionHeader = do{
name <- between (char '[') (char ']') parseName;
return name;
}
parseIniFile = do{
result <- many1 (parseSection);
return result;
}foo <- bar
return fooparseSectionHeader = between (char '[') (char ']') parseName
parseIniFile = many1 parseSectioniniFileToMap 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 xsContext
StackExchange Code Review Q#2253, answer score: 6
Revisions (0)
No revisions yet.