exercism / fsharp

Exercism exercises in F#.
https://exercism.org/tracks/fsharp
MIT License
107 stars 99 forks source link

[Squeaky Clean] new char concept and concept exercise #1248

Closed colinleach closed 5 months ago

colinleach commented 5 months ago

As discussed, this PR attempts to follow Issue #868.

It's mostly a port from C#. I had to drop the task using Char.IsControl, as F# rejects things like \0 (nul) as characters.

The about.md is much expanded, with lots more sample code. More controversially, so is introduction.md - now too much, perhaps?

colinleach commented 5 months ago

Some CI fails at the first attempt.

It dislikes the example code in my md files, in this format:

'A' < 'D'
=> val it: bool = true

I'll comment out the FSI output and resubmit.

I don't know why it thinks the .meta/config.json is "not formatted". Locally, configlet lint was happy with it, and I can't see a problem (yet).

colinleach commented 5 months ago

Docs are now fixed and passing. It still has an unexplained problem with exercises/concept/squeaky-clean/.meta/config.json

Also, I suspect the prereqs will need some attention. It only lists strings, which may not be enough. I relied on lists, the C# version relies on mutable variables (loop counter).

ErikSchierboom commented 5 months ago

I've played around a bit with the exercise, and what if we did something like this:

module SqueakyClean

open System

let transform (c: char) : string =    
    if c = '-' then
        "_"
    elif c > 'α' && c < 'ω'  then
        "?"
    elif Char.IsWhiteSpace(c) then
        ""
    elif Char.IsUpper(c) then
        "-{c.ToLower()}
    else
        c.ToString()

let clean (identifier: string): string =
    String.collect transform identifier 

This seems to check all the boxes I'd like to see ticked:

The benefits of this approach are:

The downside is:

What do you think?

colinleach commented 5 months ago

Yes! Your exercise suggestion is much better. I'll need to rewrite the instructions and tests (and take StringBuilder out of the blurb), but that should be easy. I'm 9 h behind you, so give me a day or so,

I don't feel like higher-order functions should be a dependency

For many reasons, including the small detail that we don't yet have an exercise for this, and the concept is a very short stub.

colinleach commented 5 months ago

Overall, the message I'm taking is that I should pay less attention to the C# syllabus and treat F# as a very different thing. I can't disagree.

ErikSchierboom commented 5 months ago

Overall, the message I'm taking is that I should pay less attention to the C# syllabus and treat F# as a very different thing. I can't disagree.

It might work well in some cases! But probably a combination of C# and Elixir would be good. But I'm also happy to review like this. It wasn't much work for me!

colinleach commented 5 months ago

I added a link to the higher-order-functions concept about.md, so that could usefully be expanded.

With string interpolation in there, that needs adding to log-levels as you point out.

a higher-order function is used (we should do some handwaving here, as I don't feel like higher-order functions should be a dependency)

I added a draft of this handwaving, but I suspect it could be improved.

colinleach commented 5 months ago

I hope these changes address your comments (and I didn't carelessly miss any this time). I also added a couple of minor tweaks to try and make the docs flow more coherently.