[Haskell-beginners] Code review request

[Karl Voelker]

Doug,

Here are a few thoughts:

* If you had a function "playOne :: Double -> IO ()" that plays one
  note, then you could define "play = mapM_ playOne".

* It would be nice to separate the pure logic that converts a
  frequency into a string from the IO code that prints the string and
  delays. You would probably discover this yourself if you were to
  write some unit tests.

* It seems that you are parsing and interpreting the input in a single
  pass (readSheet/readColumn). This is fine on such a small-scale
  example, although even still I find those functions a bit hard to
  grok. And a simple example like this might be a great place to learn
  about the many ways of writing parsers that are out there.

* The Ord instance for Pitch could be simplified by using the built-in
  Monoid instance for Ordering. I'm going to leave the details of that
  unspecified so you can puzzle it out.

* For the rest of the code, some comments would probably help those of
  us who are not music theory experts to follow it better. The one thing
  that caught my eye was the use of type synonyms (type Octave = Int and
  type Line = Int). You could mix up an Octave and a Line without
  getting a compile-time error. If you want to read a lot of opinions on
  when to use type synonyms, the Café had a thread about that recently:
  https://www.haskell.org/pipermail/haskell-cafe/2015-January/117771.html.

Anyway, it looks like you have learned a lot already, so good job!

-Karl
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/beginners/attachments/20150209/a99ecf22/attachment.html>