CODEWITHSUNDEEP
f#
CinnamonBun
CinnamonBun

Reputation: 1150

F# - Code improvement

i'm new to F# and i've come up with a solution to a fairly simple question, which is, given a string of numbers, I need to multiple each digit by two, then get the sum of these digits. For example, the string "123456" should equate to 24

Here is what I was able to come up with

let input = "123456"
        |> Seq.map (string >> int)
        |> Seq.map (fun(x) -> x * 2)
        |> Seq.map (int >> string)
        |> String.concat String.Empty
        |> Seq.map (string >> int)
        |> Seq.sum

My question is, is there any substantial changes I could make, to make my solution more condensed and more efficient? Any help will be greatly appreciated

Upvotes: 1

Views: 237

Answers (4)

Joel Mueller
Joel Mueller

Reputation: 28764

In terms of efficiency, instead of converting each character in the string into a string in its own right just so you can then convert it to an int...

|> Seq.map (string >> int)

I would convert the character directly to a number:

|> Seq.map (Char.GetNumericValue >> int)

This results in less garbage to collect, and although I haven't benchmarked it, I would expect it to be faster.

Upvotes: 0

Peter Ittner
Peter Ittner

Reputation: 551

Like Tomas commented I've not considered the digit-requirement in my original post. So heres my corrected version:

"123456"
  |> Seq.map    (string >> int)        // Convert to int sequence
  |> Seq.collect(fun x -> string(x*2)) // Perform multiplication, 
                                       // convert to string and collect the digits as chars
  |> Seq.sumBy  (string >> int)        // Convert the chars to string and than to integer 
                                       // to sum them correctly

It is more or less the same what Gustavo posted.

Upvotes: -1

Tomas Petricek
Tomas Petricek

Reputation: 243126

I understand that this might be a toy example, but I think that it is actually quite hard to write it in a readable way. I had to run your code line-by-line before I actually understood what is going on!

You can definitely contract it into quite short expression using point-free style (as Gustavo's answer shows), but I would probably not go that far - because you might need to be able to read & understand the code later!

An alternative way would be to use sequence expressions and write something like this:

[ for c in "123456" do
    // Get the number, multiply it & yield its characters
    let number = int (string c)
    yield! string (number * 2) ]
// Sum the numbers (converting char to int via a string)
|> Seq.sumBy (string >> int)

This is a bit shorter than your original version, but it is still (I think) fairly understandable.

Upvotes: 2

Gus
Gus

Reputation: 26204

You can compact your code by applying some properties.

fun x -> x * 2 is

fun x -> (*) x 2 (operator as a function)

fun x -> (*) 2 x (commutative)

(*) 2 (eta-reduction)

map composition is the same as the composition of the functions to map, your first 3 maps could be written as:

Seq.map (string >> int >> (*) 2 >> string)

map and then concat is collect

map and then sum is sumBy

So your code could be:

"123456"
    |> String.collect (string >> int >> (*) 2 >> string)
    |> Seq.sumBy (string >> int)

Upvotes: 11

Related Questions