F# readability test.

gistfile1.fs

let nerdCapsToKebabCase (text : string) = 
    let charToString index c =
        let s = c.ToString().ToLower()
        match index, Char.IsUpper c with
        | 0, _ -> s
        | _, true -> "-" + s
        | _ -> s
    text |> Seq.mapi charToString |> String.concat ""

Is the nerdCapsToKebabCase function readable to F# programmers?

There are lots of ways you can implement the same functionality with F#, using other map and reduce functions, but I'm not looking for suggestions for alternatives.

Currently, I'm writing an article about readability and seemingly foreign programming constructs, and I expect most readers to be unfamiliar with F#, which is the reason I went for this particular implementation, instead of one that relies heavily on Seq.collect, Seq.fold, and the like.

While I expect readers of my article to be unfamiliar with F#, I would like to be able to write that F# programmers find this function adequately readable.

Therefore, if you're an F# programmer, please leave a short comment below.

• If you find it adequately readable, you only have to write yes in your comment.
• If you don't find it readable at all, you can write no, but I'd appreciate if you also explain why.

Thank you.

Yes

Yes

Yes

Yes

Yes

No

It looks pretty good structurally, in production code I'd probably swap out the composition for a StringBuilder as the combination of Seq.mapi and String.concat are going to allocate like crazy and so it'll be pretty darn slow. In fact, I'd avoid any coercion from char to string.

To enhance readability I might write out the boolean cases at least as they're easy to enumerate and the compiler will warn you if you miss something. In this case though I don't think the match is getting you much mileage over standard if-then-else.

I know it's not pretty, but I'd probably end up writing something very C# like:

let nerdCapsToKebabCase (text : string) = 
    let sb = new StringBuilder()
    for i = 0 to text.Length - 1 do
        let c = text.[i]
        let s = Char.ToLowerInvariant(c)    
        if i = 0 then sb.Append(s) 
        elif Char.IsUpper(c) then sb.Append('-').Append(s)
        else sb.Append(s)
        |> ignore
    sb.ToString()

But it's still referentially transparent, and that's what really matters.

Yes

Yep

Yes
The only thing I don't like is the "charToString" name. It is confusing, because it's doing something more than a simple char -> string conversion (also, is the "string" type important here?). Can't think of any good alternative though ;)

Yes, I can read it, but I agree with @iskeld regarding charToString. My suggestion would be addSkewer :)

Yes, readable.

Yes.

Yes, however it took a moment for me to get it

@CraigStuntz, I totally buy the addSkewer suggestion 👍

@Rickasaurus, I also see the point in explicitly spelling out the booleans.

Now it looks like this:

let nerdCapsToKebabCase (text : string) = 
    let addSkewer index c =
        let s = c.ToString().ToLower()
        match index, Char.IsUpper c with
        | 0, _ -> s
        | _, true -> "-" + s
        | _, false -> s
    text |> Seq.mapi addSkewer |> String.concat ""

Readable for me and I've never looked at F# before.

Yes

Yes

Yes and no :-) Yes, I could read through it and understand what it was doing, so I suppose it was adequately readable. However, I couldn't intuit the intent of the code instantly, which is something I try to do, particularly for smallish functions like this - I had to do a small mental map of what each of the match clauses represented. I would've probably renamed the lower case value from "s" to something more meaningful (or removed the binding completely and just placed the same code in both branches), and possibly have created an active pattern to encapsulate the logic e.g.

let nerdCapsToKebabCase (text : string) = 
    let (|FirstLetter|UpperCase|LowerCase|) (index, c) =
        match index, Char.IsUpper c with
        | 0, _ -> FirstLetter
        | _, true -> UpperCase
        | _ -> LowerCase        

    let charToString index c =
        let s = c.ToString().ToLower()
        match index, c with
        | FirstLetter | LowerCase -> s
        | UpperCase -> "-" + s

    text |> Seq.mapi charToString |> String.concat ""

I completely agree with @Rickasaurus about efficiency, but I prefer to use more functional approach (sort of, since StringBuilder is mutable anyway):

open System
open System.Text
open FSharpx.TimeMeasurement

let nerdCapsToKebabCase (text : string) = 
    let addSkewer index c =
        let s = c.ToString().ToLower()
        match index, Char.IsUpper c with
        | 0, _ -> s
        | _, true -> "-" + s
        | _, false -> s
    text |> Seq.mapi addSkewer |> String.concat ""

let nerdCapsToKebabCase' (text: string) = 
    text.ToCharArray() |> Array.foldi (fun (sb: StringBuilder) i c ->
        if i > 0 && Char.IsUpper c then sb.Append "-" |> ignore
        sb.Append (Char.ToLowerInvariant c))
        (StringBuilder()) |> string

let str = "UaBcDeFgH12345aBcDeFaBcDeFgH12345aBcDeF"

compareTwoRuntimes
    1000000
    "Ploeh" (fun _ -> nerdCapsToKebabCase str)
    "Me" (fun _ -> nerdCapsToKebabCase' str)

Ploeh 0.010262ms
Me 0.002114ms
Ratio:  4.854304636

Yes, maybe some naming adjustment may help but I find it readable the overall code structure

Yes, but I feel in general top level functions should have return types for readability. It's not always obvious from the name (although it is here).

Yes. But I agree with @isaacabraham, his extra separation of concerns makes this kind of code slightly faster to intuit if the concepts involved (in this case kebab-case) isn't known to the reader beforehand.

Yes, readable, but distracting for me. This looks like something I could write just to get something working quickly when I'm really thinking about another problem. When reading, I keep getting distracted by the inefficiencies and will try to fix them in my head before moving on. :)

+1 @isaacabraham

Yes, it is readable. After seeing @isaacabraham 's active patterns approach I kind of like that more, but regardless, the original is clear enough.

Yes