Hello Francesco,
This is very clear. Thanks for your help !
Your version is much more readable and elegant !
Le lun. 10 févr. 2020 à 12:53, Francesco Ariis
Hello Olivier,
On Mon, Feb 10, 2020 at 10:56:40AM +0100, Olivier Revollat wrote:
I don't know if it's appropriate to post this here but I would like to have some feedback with one of my first Haskell code.
It is an appropriate post in the appropriate list!
So I decided I give it a go in Haskell, here is my solution, I appreciate if you give me some feedback on how to improve this code (make it more "idiomatic Haskell")
Ok, the problems I see with russmulList are:
russmulList :: Int -> Int -> [(Int, Int)] russmulList 1 _ = [] russmulList a b = let a' = a `div` 2 b' = b * 2 in (a', b') : russmulList a' b'
- russmulList does not handle 0 gracefully (try `russmulList 0 10`) - russmulList should _not_ discard the factors from the top of the list (or you have to awkwardly re-add them as you did in filteredPair)
This or similar will do:
russmulList :: Int -> Int -> [(Int, Int)] russmulList 0 b = [] russmulList a b = let a' = a `div` 2 b' = b * 2 in (a, b) : russmulList a' b'
Now let's go through `russmul`:
russmul :: Int -> Int -> Int russmul a b = let filteredPair = filter (\pair -> (fst pair) `mod` 2 /= 0 ) $ (a,b) : russmulList a b in foldr (\pair acc -> snd pair + acc) 0 filteredPair
- `(a,b) :` is needed no more - in filteredPair you can drop the parentheses around `fst pair` - use `odd` instead of "`mod` 2 /= 0`" - in any case you should express the predicate in point-free style as `even . fst` - `foldr` part can be made much clearer with sum (map snd ...)
So:
russmul :: Int -> Int -> Int russmul a b = let filteredPair = filter (odd . fst) (russmulList a b) in sum (map snd filteredPair)
Was this clear/useful? If not, fire again and welcome to the functional world! -F _______________________________________________ Beginners mailing list Beginners@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/beginners