-
-
Save ploeh/4f873315fc556f7672f1 to your computer and use it in GitHub Desktop.
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 "" |
@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. :)
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
Yes, however it took a moment for me to get it