Send Beginners mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
http://www.haskell.org/mailman/listinfo/beginners
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of Beginners digest..."
Today's Topics:
1. Code review request (Douglas Christman)
2. Re: Code review request (Karl Voelker)
----------------------------------------------------------------------
Message: 1
Date: Mon, 9 Feb 2015 19:36:29 -0500
From: Douglas Christman <[email protected]>
To: [email protected]
Subject: [Haskell-beginners] Code review request
Message-ID:
<canuv8qxcxxb48mbswcmuxwjoc5k_bvyyvbprglyrq-smz4c...@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Hi everyone,
After spending some time reading "Learn you a Haskell," I decided to try
writing my first Haskell program (https://github.com/dobyrch/musicbox). It
reads ASCII "sheet music" and uses the terminal bell to play the song as it
displays the frequency of each note.
I'm looking for any tips that would make my code look more idiomatic.
Please point out any places where I could simplify the logic or use
standard library functions in place of custom code. Style suggestions are
also welcome--let me know if you see anything ugly that could be written in
a more readable way.
Thanks!
Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://www.haskell.org/pipermail/beginners/attachments/20150209/473276a9/attachment-0001.html>
------------------------------
Message: 2
Date: Mon, 09 Feb 2015 21:48:55 -0800
From: Karl Voelker <[email protected]>
To: [email protected]
Subject: Re: [Haskell-beginners] Code review request
Message-ID:
<1423547335.2521317.225470565.7b4d2...@webmail.messagingengine.com>
Content-Type: text/plain; charset="iso-8859-1"
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-0001.html>
------------------------------
Subject: Digest Footer
_______________________________________________
Beginners mailing list
[email protected]
http://www.haskell.org/mailman/listinfo/beginners
------------------------------
End of Beginners Digest, Vol 80, Issue 17
*****************************************
