HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Recommend Next Note Purchase

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
recommendnotepurchasenext

Problem

I have just started trying to learn Haskell and tried to think of a practical, but simple problem to try and solve. I have a strong background in C, but am just beginning to dip my toes in the world of functional programming.

The idea came from my investing in notes on Prosper: given a list of note ratings, and a desired distribution, what should the next note purchase be, in order to most align with the desired distribution.

Here's an example:

Say I currently own 5 notes of an "A" rating, and 4 notes of a "B" rating. If my desired distribution is 50% "A" and 50% "B", then the next note purchased should be of a "B" rating. However, if my desired distribution was 90/10, the next note should be an "A" rating.

The below code seems to work (albeit with no error checking), but feels very unreadable. What are some ways I could refactor to make the code more readable or idiomatic to Haskell?

module Main where

import Data.List
import Data.Ord

main::IO()
main = print $ recommendNote [ "B", "C", "D", "D", "HR" ]  [ 0, 0, 0, 0.5, 0.25, 0.25, 0]

noteTypes :: [String]
noteTypes = [ "AA", "A", "B", "C", "D", "E", "HR" ]

recommendNote :: (Ord a, Fractional a) => [String] -> [a] -> String
recommendNote notes targetDist = let y = zip (subtractLists targetDist (getDistribution notes)) noteTypes
                                in snd (maximumBy (comparing fst) y)                            

subtractLists :: (Num a) => [a] -> [a] -> [a]
subtractLists = zipWith (-)

count :: [String] -> String -> Int
count inList x =  length $ filter (x==) inList

getNoteCount :: [String] -> [Int]
getNoteCount inList = map (count inList) noteTypes 

getDistribution :: (Fractional a) => [String] -> [a]
getDistribution inList = percentize $ map fromIntegral (getNoteCount inList)

percentize :: (Fractional a) => [a] -> [a]
percentize inList = map (/ sum inList) inList

Solution

Order of arguments

As a convention, arguments in functions are always ordered from most likely to change to less likely to change.

Because this allows...

Simplification (Currying)

Haskell is like Math.

Look at

2x + x = 1 + x


You would like the simplify the x, so:

2x = 1


The same applies to your code:

percentize inList = map (/ sum inList) inList


You can omit inList as it is just repeated on both sides:

count x inList =  length $ filter (x==) inList


Becomes:

count x = length . filter (x==)


Now the focus is not on the transformation of the argument, but on the definition of a function as the successive application of two (or more) known ones.

Another example is:

getDistribution inList = percentize $ map fromIntegral (getNoteCount inList)


That becomes:

getDistribution = percentize . map fromIntegral . getNoteCount


This is, left to right, a description of what the function does.

Conventional names

I see that inList goes inside the function, and I can see that it is a List from the type signature, such long names are not needed.

Just use the conventional xs for general lists:

percentize xs = map (/ sum xs) xs


Use where not let

It is more natural to read from general to specific than the opposite:

recommendNote notes targetDist = let y = zip (subtractLists targetDist (getDistribution notes)) noteTypes
                                in snd (maximumBy (comparing fst) y)


Becomes:

recommendNote notes targetDist = snd (maximumBy (comparing fst) y)
  where
    y = zip (subtractLists targetDist (getDistribution notes)) noteTypes


Where you also avoid overly long lines.

Generality

Be optimistic and general with your types:

count :: String -> [String] -> Int


Becomes:

count :: a -> [a] -> Int


Where a stands for anything.

Not everything is get

get is a specific OO technical term, prefixing it before the function names is confusing, just drop it.

Make tiny functions local

Things like:

subtractLists = zipWith (-)


May be local and without a type declaration, like:

recommendNote notes targetDist = snd (maximumBy (comparing fst) y)
  where
    y = zip (subtractLists targetDist (getDistribution notes)) noteTypes 
    subtractLists = zipWith (-)

Code Snippets

2x + x = 1 + x
percentize inList = map (/ sum inList) inList
count x inList =  length $ filter (x==) inList
count x = length . filter (x==)
getDistribution inList = percentize $ map fromIntegral (getNoteCount inList)

Context

StackExchange Code Review Q#113989, answer score: 3

Revisions (0)

No revisions yet.