Recalling the problem, I have something like 8 different kinds of input files that I read into GenI and my code was a bit of a mess. It was heavily factored, but somehow in the wrong way. Something very fragile and useless feeling about it. Some things that vary:
- most files read lists of things, but some deal with more exotic objects
- most files are text based, but some have the option of some binary encoding
- most files are with parsed with Parsec, some others with simple line processing, or with JSON
- some files are "mandatory" - if we try to load them and you don't supply them as input, something bad should happen; some files are optional - trying to load them should be a no-op if you don't
change 1: use a typeclass
I haven't changed anything fundamental about how the code works so much as restructure it. The first big difference is that I now have a typeclass that captures the 3 main things GenI does when it loads some file:class Loadable x where lParse :: FilePath -- ^ source (optional) -> String -> Either ParseError x lSet :: x -> ProgState -> ProgState lSummarise :: x -> String
Having this typeclass has made my code a lot more uniform. The old approach was to have a generic higher order load function that took lots of other functions and various bits and bobs as input (so it was factored but incomprehensible). Now I only need to specify that the format I'm loading in is Loadable, and it's implicit that I have access to these three functions.
The main function here is
lParse. There are some implicit decisions here. First, parsing should be treated as something that's done on strings, and not files. I'm not sure if I'll come to regret this. In the future, I may update this decision so that parsing is done on some sort of binary blob instead, but with the core idea that this excludes any of the IO behind it. Second, parsing a file and massaging it is treated as a single cohesive black box. My various parsers used to scatter this in different places, mainly in the bit that updates my program state.The other functions,
lSet and lSummarise are not so important. They update the program state and provide a count string respectively, something along the lines of "3 elementary trees".change 2: more composable helpers
Instead of having one big load helper function, I've broken down some of the choices into standalone functions that you can then mix and match.I now have
withFlagOrIgnoreandwithFlagOrDie: extracts the value of some flag and respectively silents ignores if it's not there, or just exits the programwithLoadStatus: takes a file parsing function and decorates it with "now loading X" and "...n items loaded" output (using thelSummarisefunction)throwOnParseError: takes a parse output and either throws an exception if there is a parse error or returns the valuelParseFileandlSetState: IO bound wrappers aroundLoadable
Having these functions separated from each other makes it easier for me to modify behaviour, and also helps me to avoid redundancy when I have subtly different requirements (like the binary file thing). I can just mix and match them in slightly different ways. One change that this facilitated was for me to replace a
dieOnParseError with throwOnParseError, just a straight shot darcs replace and a tweak on the function.So, GenI file loading now a lot better. Not going to win any awards or be a Functional Pearl, but at least it's less horrible than before.




