Using Fibonacci sequence to generate musical melodies

Question

I recently started picking up the Haskell programming language. I've managed to learn other languages rather quickly, but I'm still having a difficult time grasping some of the basics of Haskell.

I decided to practice and wrote a program that calculates the Fibonacci Sequence (original, I know) a certain amount of times and adds the results to a list, then divides each number in the list by 7 and creates a new list of the remainders of that division. It then assigns a musical note letter based in A minor (A, B, C, D, E, F, and G), and prints out each letter. The idea was to give me new ideas for writing melodies.

The program works, but with how efficient Haskell is I feel like it's way too long for what it does. How would you guys suggest re-writing this and what are some shortcuts I can use to write better code?

Here's the program:

import Data.List
import System.IO
fibo a b = a:fibo b (a+b)
divBy7 x = x `mod` 7
calcNums = take 16 $ map divBy7(fibo 0 1)
returnNote :: Int -> String
returnNote n
 | (n == 0) = "A"
 | (n == 1) = "B"
 | (n == 2) = "C"
 | (n == 3) = "D"
 | (n == 4) = "E"
 | (n == 5) = "F"
 | (n == 6) = "G"
-- the number will never be higher than 6 due to the pisano period
output = map returnNote(calcNums)

I'm just running ghci and asking it to return output

• Frerich Raabe: If you pull the "modulo 7" step into returnNote, you would no longer need the otherwise guard and hence define returnNote n = "ABCDEFG" !! (n `mod` 7).
• Frerich Raabe: There's also no need for the $ function in the definition of output.

Answer 1

fibo

fibo is a fine function, but I would suggest a more complete name like fibonacci just to make it 100% clear that it returns fibonacci numbers to someone that did not read your plaintext introduction.

divBy7

divBy7 is not a good function: it is trivial and most importantly does not do what it says. It returns the remainder of dividing by 7, not the result of dividing by 7. I would delete this function.

calcNums

calcNums is not general enough, I feel no need for the take 16 at the start of it, this function has the potential to calculate as many fibonacci mod 7 as you want, let the caller decide how many to take.

Incidentally fibonacciMod7 is a better name for the function as it says what it does: calcNums was way too general.

returnNote

returnNote is too verbose and fails badly (= with no meaningful error message) if I pass it anything bigger than 6.

returnNote n 
 | n `elem` [0..6] = "ABCDEFG" !! n
 | otherwise = error "n will never be higher than 6 due to the pisano period. (And it must be positive)"

This implementation is more concise and incorporates the comment into the code so that it can be shown to the user when he uses the function incorrectly.

• Jake Rieger: Despite not describing it on my part, the take 16 is used because when dividing each number in the sequence by 7, the remainders repeat every 16 numbers so taking more is useless. If you read the comment, the number will never exceed 6, and since there are only 7 notes in a musical scale (if we're taking results 0 to 6), then why pass anything bigger than 6? I agree with everything else you mentioned.
• Caridorc: @JakeRieger Taking less numbers is still reasonable. I think the error is better than the comment because it is in the code and is more standardized and testable. Also when testing interactively to learn the program the error can make the tester learn the expected condition faster.
• Jake Rieger: I tend to have a habit writing code that does purely what I want it to and tend to ignore good practices like accurate variable names and what not. Thanks for reminding me to write good code regardless of who's gonna see it.
• 200_success: If you roll the mod 7 into returnNote as @FrerichRaabe suggests, then there would be no error case, and no need for a weirdly specific fibonacciMod7 function.
• Caridorc: @JakeRieger fibonacciMod7 is not to be deleted, just the take 16 in front of it is. You could put it in output: output = map returnNote (take 16 fibonacciMod7)

Answer 2

You can greatly reduce the code in returnNote by using the list index operator !!.

returnNote n = ["A", "B", "C", "D", "E", "F", "G"]!!n

• Caridorc: Exactly, I had written this into my answer but was revising other parts before posting. Nice confirmation to know we had the same idea independently.

Answer 3

you could shortcut it to sumthin like

ghc -e 'take(16)$ 
 map(("ABCDEFG"!!).(`mod`7))$
 fix((0:).scanl(+)1)'

likewise

fibos a b = a : fibos b (b + a) 
music = fibos 0 1
note = ("ABCDEFG"!!).(`mod`7)
notes = map note music
melody = take 16 notes

• Jake Rieger: Compared to the other suggestions here, this is a rather ugly solution. I'm looking for ways to improve my code, not make it uglier and harder to read. Thanks for your input though.