Follow the Haskell convention of names
The Haskell convention is camelCase. That is, start names with a
lowercase letter, and join words together by capitalizing them. So
head_lens would be
headLens, or better yet,
Use names with meanings, not just labels
Come to think of it, an even better name would be
as that is not just what the value is, but what it means (and
what it is used for).
erg don't tell me a lot,
preprocess I would suggest a name like
rightPadLines. That way, I won't be so surprised to find out what it does :)
Don't be afraid of longer names
It is also useful and nice for other readers not to use
abbreviations, unless the scope is very small. So the
dissect line are good and clear (very short scope -> very
add_del, on the other hand, would be more readable
delim is a somewhat more conventional
Use the type system
Yes, Haskell can find out what types your values and functions
have. And it will be correct in many, many cases. But if you
are wrong, Haskell cannot tell you, unless you tell the compiler
what you intend to do. So always, always write the type signatures
of at least the top-level bindings. That way, if your function is
not yet finished, or you make a mistake, the compiler will be able
to tell you almost exactly where.
main :: IO ()
main = do -- ..
rpad :: Int -> String -> String
rpad len string = take -- ..
preprocess :: [String] -> [String]
preprocess lines = map -- ..
dissect :: String -> [Int]
compress :: [String] -> [[String]]
inflate :: [String] -> [[String]]
Adding those type signatures alone, makes the function more
readable, because now I can expect what it is going to do.
Use hlint on the code and heed warnings from ghc
Many warnings and suggestions are worthwile. They can point out
possible mistakes or partial functions, but also show what an
idiomatic Haskell solution could be. For instance:
head instead of
readfile (args !! 0) is reminiscent of other languages, where the
command line arguments are in an array, which you'd need to index.
Using the 0th index though, is normally done by using the
readFile $ head args is the suggested alternative.
elem is more readable as
Functions that take more than one argument, can be inserted between
the first and second argument (infix) if you write backquotes
around them. For many functions, such as
this makes much sense. So
if elem "inflate" args can be
if "inflate" `elem` args.
In that same line,
(compress) have parentheses
around them, but they are really not necessary. Removing them
makes the line more readable (even for LISP programmers).
The same goes for
map (length) lines in
map (shrinkcol) cols in
(map length) c in
string++(repeat ' ') can do without the parentheses.
Three functions use the name
lines for a parameter, which happens
to be the name of a function, too. It is best to heed this
warning, for two reasons:
- If you happen to want to split some string later on in those
functions, you'll have to change the name anyway.
- People who see
lines somewhere, will probably think of the
function first, be confused, then remember that it is a
ls is not that bad, by the way, especially if you change
the type to be more documentational (see below).
This mathematical term comes down to "remove the variable name", so
that the focus lies more on what the function does, and less of
what it functions on. The function
shrinkcol has a
c on both
ends, and they can both be removed, resulting in:
shrinkcol = map (\el -> reverse $ dropWhile (==' ') $ reverse $ dropWhile (==' ') el)
Which expresses exactly the same, just in less "words".
This warning, on the same function as above, suggests about the
same thing, but now for the
\el variable. However, because we
$ for application of values, this line should be
. for composition. As
f . g means: do
g, this comes mostly down to just replacing the
shrinkcol = map (reverse . dropWhile (==' ') . reverse . dropWhile (==' '))
Another composition opportunity
The last warning that ghc and hlint give us, is to use composition
mapM_ call. Instead of saying: "map
putStrLn over the
result of mapping
concat over the list
erg, and combine that
IO action", you can say "map (
over the list
erg, and combine that into one
IO action". Which
is shorter in English already :)
mapM_ (putStrLn . concat) erg
And now we are warning-free!
Use the type system
What, again? Yes. But this time, use it to document the intent of
the functions, not just the functionality. The easiest way to do
this, is to declare simple type aliases. For instance,
preprocess works on lists of
String, but each
Line. So adding
type Line = String, while it does
not give any added compiler safety (
Line is just an alias, and
exactly the same as
String), it does give a bit of documentation.
type Line = String
type Cell = String
preprocess :: [Line] -> [Line]
compress :: [Line] -> [[Cell]]
inflate :: [Line] -> [[Cell]]
Of course, making stronger types with constructors, using
even better, so that the type system helps you check whether the
code works, even before you run it. Then, you can write functions
as transformations between types, instead of transformations