2
\$\begingroup\$

I decided to play with F# and found simple task on Codewars. The task is sounds like "Calculate Sum/Mean/Avarage for selected City". I decided to calculate only Sum. My solution is horrific. I would be grateful for your advice's how to do this in a right way. Thank you!

let d0 = 
        "Rome:Jan 81.2,Feb 63.2,Mar 70.3,Apr 55.7,May 53.0,Jun 36.4,Jul 17.5,Aug 27.5,Sep 60.9,Oct 117.7,Nov 111.0,Dec 97.9\n\
        London:Jan 48.0,Feb 38.9,Mar 39.9,Apr 42.2,May 47.3,Jun 52.1,Jul 59.5,Aug 57.2,Sep 55.4,Oct 62.0,Nov 59.0,Dec 52.9\n\
        Paris:Jan 182.3,Feb 120.6,Mar 158.1,Apr 204.9,May 323.1,Jun 300.5,Jul 236.8,Aug 192.9,Sep 66.3,Oct 63.3,Nov 83.2,Dec 154.7\n\
        NY:Jan 108.7,Feb 101.8,Mar 131.9,Apr 93.5,May 98.8,Jun 93.6,Jul 102.2,Aug 131.8,Sep 92.0,Oct 82.3,Nov 107.8,Dec 94.2\n\
        Vancouver:Jan 145.7,Feb 121.4,Mar 102.3,Apr 69.2,May 55.8,Jun 47.1,Jul 31.3,Aug 37.0,Sep 59.6,Oct 116.3,Nov 154.6,Dec 171.5\n\
        Sydney:Jan 103.4,Feb 111.0,Mar 131.3,Apr 129.7,May 123.0,Jun 129.2,Jul 102.8,Aug 80.3,Sep 69.3,Oct 82.6,Nov 81.4,Dec 78.2\n\
        Bangkok:Jan 10.6,Feb 28.2,Mar 30.7,Apr 71.8,May 189.4,Jun 151.7,Jul 158.2,Aug 187.0,Sep 319.9,Oct 230.8,Nov 57.3,Dec 9.4\n\
        Tokyo:Jan 49.9,Feb 71.5,Mar 106.4,Apr 129.2,May 144.0,Jun 176.0,Jul 135.6,Aug 148.5,Sep 216.4,Oct 194.1,Nov 95.6,Dec 54.4\n\
        Beijing:Jan 3.9,Feb 4.7,Mar 8.2,Apr 18.4,May 33.0,Jun 78.1,Jul 224.3,Aug 170.0,Sep 58.4,Oct 18.0,Nov 9.3,Dec 2.7\n\
        Lima:Jan 1.2,Feb 0.9,Mar 0.7,Apr 0.4,May 0.6,Jun 1.8,Jul 4.4,Aug 3.1,Sep 3.3,Oct 1.7,Nov 0.5,Dec 0.7"


module CalculatorHelper =
    let SumByTown (town:string) (input:string) =
        let rows = input.Split('\n')
        for i in rows do
            if (i.Contains(town))
            then 
                let monthes = i.Replace(town + ":","").Split(",")
                let sequence = seq { for j in monthes do yield j.Remove(0,4) |> double } |> Seq.sum
                printf "%f" sequence


let result = CalculatorHelper.SumByTown "Rome" d0
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

You should reduce the use of explicit type declaration as much as possible.


A function isn't of much value if it just prints its calculated result. You should return the result to let it be up to the client to handle it.

But you can't break out from a loop via an if-statement in F#. You can overcome that by wrapping the outer for-loop in a seq {...} statement and then yield the one and only result and then take the first element from that sequence. But that is inefficient in that it iterates through all the cities even if the first is the one searched for.

Below I've refactored your algorithm according to the above in order to be able to return the result from the function:

let SumByTown town input =
    let rows = (string input).Split('\n')
    seq { for row in rows do
            if (row.Contains(town)) then 
                let months = row.Replace(town + ":" , "").Split(',')
                let sequence = seq { for j in months do yield j.Remove(0,4) |> double } |> Seq.sum
                yield sequence
    } |> Seq.head

But this still doesn't look very functional IMO.


For inspiration is here a version that uses the built in higher order functions in a more functional manner:

let calculateCity city data =
    let sum values = values |> Array.sum
    let avg values = values |> Array.average
    let mean values = (values |> Array.sort).[values.Length / 2]

    let handleCity cityText = 
        let values = Regex.Replace(cityText, "[^0-9,.]+", "").Split(',') |> Array.map float
        (sum values, avg values, mean values)

    (string data).Split('\n') |> Seq.find (fun s -> s.StartsWith(city)) |> handleCity

let printCityData (sum, avg, mean) = printfn "Sum: %.2f, Average: %.2f, Mean: %.2f" sum avg mean

Usage:

printCityData (calculateCity town data)

Update

Antoher way to use Regex is to match numbers instead of removing non-numeric chars:

let calculateCity1 city data =
    let sum values = values |> Seq.sum
    let avg values = values |> Seq.average
    let mean values = (values |> Seq.sort).ElementAt((values|> Seq.length)/ 2)
    let extractValues text = Regex.Matches(text, "\d+.\d+").Cast<Match>() |> Seq.map (fun m -> m.Value)

    let handleCity cityText = 
        let values = cityText |> extractValues |> Seq.map float
        (sum values, avg values, mean values)

    (string data).Split('\n') |> Seq.find (fun s -> s.StartsWith(city)) |> handleCity

let printCityData (sum, avg, mean) = printfn "Sum: %.2f, Average: %.2f, Mean: %.2f" sum avg mean

This requires an open System.Linq at the top of the module

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.