Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am choosing to learn F# for my own enjoyment. I am getting to the point where concepts of F# seem to be pretty easy, but understanding some of the whys and whens is a bit harder.

Before I get into the code and the explanation, let me put my question up front. I am asking where can I find better advice on formatting F# code for readability? Or can someone give me a few guiding tips based off of the example given below?

So what are the best practices for code format and file layout?

Now to the explanation of the code.

I have started practicing coding Kata's in F# just to allow me to flex the language a little. The bellow program is an implementation of a natural sort. This was my first attempt to solving the problem in a TDD fashion in F#, as such I chose to forgo any framework as I did not want to deal with figuring out how to use any of them and not break the functional paradigm.

So the code below carries a light weight unit test framework.

Here is the code for the natural sort:

namespace Katas
    open System.Linq

    module NaturalSortKata =
        exception InvalidException of string

        type Comparison =
        | Equal
        | Lesser
        | Greater
            static member Compare x y =
                if x = y then
                    Equal
                elif x > y then
                    Greater
                else
                    Lesser

        type ChunckType =
        | NumberType
        | StringType
        | Unknown
            static member GetType (c : char) =
                if System.Char.IsDigit(c) then
                    NumberType
                else
                    StringType

            member this.Compare other = 
                match other with
                | ty when ty = this -> Equal
                | Unknown -> Lesser
                | NumberType when this = Unknown -> Greater
                | NumberType -> Lesser
                | StringType -> Greater

        let natualCompare (left : string) (right : string) = 
            if left = right then
                Equal
            else
                let fix str =
                    new System.String( str |> List.rev |> List.toArray )

                let gatherChunck str = 
                    let rec gather str acc =
                        match str with
                        | [] ->
                            let (ty, l) = acc
                            (ty, fix(l))
                        | fistLetter::rest ->
                            match acc with
                            | (ty, _) when ty = Unknown ->
                                let t = ChunckType.GetType(fistLetter)
                                gather rest (t, fistLetter :: [])
                            | (ty, l) when ty = ChunckType.GetType(fistLetter) ->
                                gather rest (ty, fistLetter::l)
                            | (ty, l) -> (ty, fix(l))

                    gather str (Unknown, [])

                let rec compare (left : string) (right : string) =
                    if (not (left.Any())) || (not (right.Any())) then
                        match left.Length, right.Length with
                        | llen, rlen when llen = rlen -> Equal
                        | llen, rlen when llen > rlen -> Greater
                        | llen, rlen when llen < rlen -> Lesser
                        | _ -> raise (InvalidException "Bad Data")
                    else
                        let lt, lChunk = left |> Seq.toList |> gatherChunck 
                        let rt, rChunk = right |> Seq.toList |> gatherChunck

                        match lt.Compare rt with
                        | Equal ->
                            if lChunk = rChunk then
                                let lVal = left.Replace(lChunk, "")
                                let rVal = right.Replace(rChunk, "")

                                compare lVal rVal
                            else
                                match lt with
                                | NumberType ->
                                    Comparison.Compare (System.Int64.Parse(lChunk)) (System.Int64.Parse(rChunk))
                                | _ ->
                                    Comparison.Compare lChunk rChunk
                        | _ ->
                            lt.Compare(rt)

                compare left right

Here is the code for the tests:

namespace Katas.Testing
    open Katas.NaturalSortKata

    module Tests =
        let test left right expected title=
            let testRun x =  
                let result = right |> natualCompare left
                if result = expected then
                    x |> printfn "%d good"
                    true
                else
                    title + " fails" |> printfn "%d %s" x
                    result 
                    |> sprintf "%d            Actual: %A" x 
                    |> sprintf "%d            Expected: %A\r\n%s" x expected
                    |> printfn "%s"
                    false

            testRun

        let testRunner tests=
            let rec runner x result tests =
                match tests with
                | [] -> result
                | head::tests -> 
                    let current = (head x) && result
                    tests |> runner (x + 1) current

            tests |> runner 1 true

        let test01 = "Simple Equality"                           |> test "one"               "one"               Equal
        let test02 = "left < right"                              |> test "left"              "right"             Lesser
        let test03 = "beta > alpha"                              |> test "beta"              "alpha"             Greater
        let test04 = "\"9\" < \"10\""                            |> test "9"                 "10"                Lesser
        let test05 = "\"alpha9\" < \"alpha10\""                  |> test "alpha9"            "alpha10"           Lesser
        let test06 = "\"alpha9Centary9\" < \"alpha9Centary10\""  |> test "alpha9Centary9"    "alpha9Centary10"   Lesser
        let test07 = "\"10\" > \"9\""                            |> test "10"                 "9"                Greater
        let test08 = "\"alpha10\" > \"alpha9\""                  |> test "alpha10"            "alpha9"           Greater
        let test09 = "\"alpha9Centary9\" < \"alpha9Centary10\""  |> test "alpha9Centary10"    "alpha9Centary9"   Greater

        let tests = test01 :: test02 :: test03 :: test04 :: test05 :: test06 :: test07 :: test08 :: test09 ::[]


        let runTests =        
            tests |> testRunner |> printfn "%b"

Here is the code that runs it all:

open Katas.NaturalSortKata
open Katas.Testing.Tests

[<EntryPoint>]
let main argv = 
    //Katas.Lockers.showLockerResults 300
    runTests
    let _ = System.Console.ReadKey(true)
    1

Thank you very much for whatever advice you can give.

share|improve this question
    
BTW, I believe the idiomatic way of ignoring the result of something is System.Console.ReadKey(true) |> ignore, not declaring an unused local. Also, I think you don't even need to declare main, you can put the code directly at the top level of the file. –  svick Jan 27 '13 at 13:40
    
Also, “chunck” is a typo, the correct spelling is “chunk”. –  svick Jan 27 '13 at 13:41
    
I did not know about ignore. Thank you. I started learning functional programming through Erlang, which an underscore is an ignore. I assumed that it would be the same. –  JasonKerney Feb 1 '13 at 4:37
add comment

migrated from stackoverflow.com Jan 27 '13 at 13:03

This question came from our site for professional and enthusiast programmers.

1 Answer

up vote 6 down vote accepted

Your code formatting is very readable. The minor issue is that you don't have to indent after namespace declaration. I would choose to open module directly to save a level of indentation (see attached code later).

So what are the best practices for code format and file layout?

I think F# code formatting guidelines have very concrete suggestions on these issues.

There are a few small problems with your code:

  1. In naturalCompare, you should move local functions to the top so the logic of the function is clear.

  2. In compare, if you remove when in the last clause, you don't have to superficially throw an exception.

  3. Value tests is more readable in the form of normal list declaration.

    let tests = [ test01; test02; test03; test04; test05; test06; test07; test08; test09; ]
    

Here is reformatted version of Katas.NaturalSortKata module. You could do the same for Katas.Testing.Tests.

// 0) Declare module to save a level of indentation
module Katas.NaturalSortKata

open System.Linq

type Comparison =
| Equal
| Lesser
| Greater
    static member Compare x y =
        if x = y then
            Equal
        elif x > y then
            Greater
        else
            Lesser

type ChunckType =
| NumberType
| StringType
| Unknown
    static member GetType (c : char) =
        if System.Char.IsDigit(c) then
            NumberType
        else
            StringType

    member this.Compare other = 
        match other with
        | ty when ty = this -> Equal
        | Unknown -> Lesser
        | NumberType when this = Unknown -> Greater
        | NumberType -> Lesser
        | StringType -> Greater

let naturalCompare (left : string) (right : string) = 
    // 1) Move local functions on top
    let fix str =
        new System.String( str |> List.rev |> List.toArray )

    let gatherChunck str = 
        let rec gather str acc =
            match str with
            | [] ->
                let (ty, l) = acc
                (ty, fix(l))
            | fistLetter::rest ->
                match acc with
                | (ty, _) when ty = Unknown ->
                    let t = ChunckType.GetType(fistLetter)
                    gather rest (t, fistLetter :: [])
                | (ty, l) when ty = ChunckType.GetType(fistLetter) ->
                    gather rest (ty, fistLetter::l)
                | (ty, l) -> (ty, fix(l))

        gather str (Unknown, [])

    let rec compare (left : string) (right : string) =
        if (not (left.Any())) || (not (right.Any())) then
            match left.Length, right.Length with
            | llen, rlen when llen = rlen -> Equal
            | llen, rlen when llen > rlen -> Greater
            | llen, rlen -> Lesser // 2) Remove superficial when guard
        else
            let lt, lChunk = left |> Seq.toList |> gatherChunck 
            let rt, rChunk = right |> Seq.toList |> gatherChunck

            match lt.Compare rt with
            | Equal ->
                if lChunk = rChunk then
                    let lVal = left.Replace(lChunk, "")
                    let rVal = right.Replace(rChunk, "")

                    compare lVal rVal
                else
                    match lt with
                    | NumberType ->
                        Comparison.Compare (System.Int64.Parse(lChunk)) (System.Int64.Parse(rChunk))
                    | _ ->
                        Comparison.Compare lChunk rChunk
            | _ ->
                lt.Compare(rt)

    if left = right then
        Equal
    else
        compare left right

UPDATE:

After answering your question, I started developing a source code formatter for F#. More information can be found on Gihub.

Results from the tool or its companion formatting guideline can give you some hints for good formatting.

share|improve this answer
    
Thank you, the link is great, and the advice you give makes since. –  JasonKerney Jan 29 '13 at 17:26
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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