Michael Mossey wrote:
I'm wondering if I could get really general comments on my program... like notes about language features I'm failing to take advantage of.
This is part of a program to convert MusicXML to a simpler representation which I then manipulate. Most of the data is defined in MusDoc.Data:
http://hpaste.org/fastcgi/hpaste.fcgi/view?id=27679#a27679
and the functions are defined in MusDoc.FromXml:
http://hpaste.org/fastcgi/hpaste.fcgi/view?id=27678#a27678
A module called XmlAccess can access various interesting parts of the MusicXML document in a way that lets me ignore most of its complexity:
I'm not familiar enough with the problem domain to say anything about the large scale organization of your code. But on a per-function scale, you have a few helper functions that are best expressed with list comprehension and combinators like fold or listToMaybe instead of general recursion. For instance, I recommend to write -- in module MusDoc.XmlAccess headerScoreParts = ... where remainder = [part | Part_List_2 scorePart <- pl_s] -- in module MusDoc.FromXml xScorePart = ... where ... lookupMidiInstr scInstrId = listToMaybe . filter ((scInstrId==) . A.midiInstrumentId) In a sense, the Haskell Prelude and standard libraries come with a superb "domain specific language for manipulating (some) lists" which is much easier to understand and visualize than general recursion. :) Concerning the MusicXML package itself, I think the use of 7-tuples in the MusicXML package for data types like Score_Header is creepy. I do admit that it has some interesting, yet ghastly appeal due to its regularity, but frankly, I think this is bad design. For instance, consider the following randomly chosen example data Orientation_ = Orientation_1 | Orientation_2 deriving (Eq, Show) read_Orientation_ :: Data.Char.String -> Result Orientation_ read_Orientation_ "over" = return Orientation_1 read_Orientation_ "under" = return Orientation_2 I am rather dissatisfied that the constructors are named Orientation_1 and Orientation_2 instead of Over and Under , and that read_Orientation is not a member of a type class or thelike. I can only hope that this code was generated automatically ... Regards, Heinrich Apfelmus -- http://apfelmus.nfshost.com