Hi Olivier,

I had a quick look, I think it's a great first try. Here are my thoughts!

module DivRusse where

{-

  Comments:

  * This seems unsafe in that it doesn't handle negative numbers well.

  * This can be evidenced by needing a guard on our property.

  * Could be addressed by using a safe newtype.

  * Define properties and use quickcheck to test them.

  * Favor pattern-matching over use of `fst`, `snd`.

  * Use `where` over `let` to highlight what the final result is.

  * Rewrite folds to more wholemeal approach. e.g. `sum $ map snd filteredPair`

  * Use standard functions and composition to eliminate lambdas like this: `(\(x, _) -> x `mod` 2 /= 0 )` = `(odd . fst)`.

  * `russmulList` could go into an infinite loop for negative numbers. Either prevent this with types (preferred), or return an error somehow.

 -}

main :: IO ()

main = do

  putStrLn "13 x 12 is"

  print $ russmul 13 12

-- Property: Does russmul = *?

prop_russmul :: Int -> Int -> Bool

prop_russmul a b

  | a > 0 && b > 0 = russmul a b == a * b

  | otherwise      = True

russmul :: Int -> Int -> Int

russmul a b  = sum $ map snd filteredPair

  where

    filteredPair = filter (odd . fst)  $ (a,b) : russmulList a b

russmulList :: Int -> Int -> [(Int, Int)]

russmulList 1 _ = []

russmulList a b = (a', b') : russmulList a' b'

  where

    a' = a `div` 2

    b' = b * 2

Warm Regards,

 - Lyndon

On Mon, Feb 10, 2020 at 8:55 PM Olivier Revollat <revollat@gmail.com> wrote:

Hi everybody,

it's my first message on this ML :)

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.

I've been inspired by a recent Numberphile video (https://www.youtube.com/watch?v=HJ_PP5rqLg0) how explain the "Russian Peasant" algorithm to do multiplication (here in a nutshell : https://www.wikihow.com/Multiply-Using-the-Russian-Peasant-Method)

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")

NB : I apologize if it's not the right place to ask this kind of review ... in that case, where can I post this ?

Thanks !

module DivRusse where

main :: IO ()

main = do

putStrLn "13 x 12 is"

print $ russmul 13 12

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

russmulList :: Int -> Int -> [(Int, Int)]

russmulList 1 _ = []

russmulList a b =

let a' = a `div` 2

b' = b * 2

in (a', b') : russmulList a' b'

_______________________________________________
Beginners mailing list
Beginners@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/beginners

_______________________________________________
Beginners mailing list
Beginners@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/beginners