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