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

colinleach · 2024-04-18T19:31:48Z

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 · 2024-04-18T19:46:50Z

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 · 2024-04-18T20:18:40Z

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

Thanks for this! I think the key thing is to get the exemplar right. I feel like it might be a tid too complex for this exercise, so we might need to tweak the exercise a bit.

concepts/chars/.meta/config.json

ErikSchierboom · 2024-04-19T08:41:44Z

exercises/concept/squeaky-clean/.meta/config.json

@@ -0,0 +1,18 @@
+{
+  "authors": ["colinleach"],


This is what configlet fmt should be fixing. Try running bin/configlet fmt --update --yes

exercises/concept/squeaky-clean/.docs/introduction.md

ErikSchierboom · 2024-04-19T08:49:25Z

config.json

+        "prerequisites": [
+          "strings"


I agree that we probably want to add to this, but let's first get the exemplar implementation right.

concepts/chars/introduction.md

ErikSchierboom · 2024-04-19T08:59:43Z

concepts/chars/introduction.md

+To append a `char` to a `string`, convert it to a string first:
+
+```fsharp
+"Part" + string '1'
+// => val it: string = "Part1"
+```


This is very uncommon and hardly ever used I think. Previously, one would use sprintf for this, but nowadays string interpolation would be the way to go. Maybe use that as an example?

BTW, I noticed that we don't yet mention string interpolation in https://github.com/exercism/fsharp/blob/main/exercises/concept/log-levels/.docs/introduction.md, which might make sense. We could then have a separate exercise dedicated to string interpolation and its intricacies.

To append a char to a string, convert it to a string first

As you maybe guessed, this was a half-assed last-minute addition that I sensibly left out of earlier drafts. Trying/failing to make the exercise work early in the syllabus when the student doesn't know much.

ErikSchierboom · 2024-04-19T08:59:55Z

concepts/chars/introduction.md

+Using a StringBuilder may be more performant:
+
+```fsharp
+let sb = new System.Text.StringBuilder()
+// => val sb: System.Text.StringBuilder = 
+
+sb.Append('F').Append('#')
+// => val it: System.Text.StringBuilder = F# {Capacity = 16; Chars = ?; Length = 2; MaxCapacity = 2147483647;}
+
+sb.ToString()
+// => val it: string = "F#"
+```


I have to think about this a bit more. StringBuilder is very useful but also a bit overkill for most use cases.

ErikSchierboom · 2024-04-19T09:00:38Z

concepts/chars/introduction.md

+Converting between a `list` of `char` and a `string` is simple:
+
+```fsharp
+System.String.Concat ['E'; 'x'; 'e'; 'r'; 'c'; 'i'; 's'; 'm']
+// => val it: string = "Exercism"
+
+"Exercism" |> Seq.toList
+// => val it: char list = ['E'; 'x'; 'e'; 'r'; 'c'; 'i'; 's'; 'm']
+```


This introduces lists, which we would need to have as a prerequisite (might be fine).

I'm increasingly thinking that lists would be a good prereq, moving chars down the syllabus.

ErikSchierboom · 2024-04-19T09:26:30Z

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:

Use character literals ('-', 'α' and 'ω')
Use static System.Char methods (like IsWhiteSpace())
Use System.Char methods (like ToUpper())
Compare char instances via =, < and >
Convert from char to string
"Append" a char to a string via string interpolation (or sprintf for now, until we've updated the log-levels exercise)
Show that strings are a sequence of characters (albeit a bit indirectly)

The benefits of this approach are:

We don't have to get in mutable territory, which a for loop would force us into
We don't need StringBuilder
We don't need pattern matching, but can instead just use a regular if-else (we need the "if-then-else-expressions" prerequiste)
We have students use the String module

The downside is:

We don't have student's use a for loop, but instead a higher-order function is used (we should do some handwaving here, as I don't feel like higher-order functions should be a depedency)

What do you think?

Co-authored-by: Erik Schierboom <[email protected]>

colinleach · 2024-04-19T13:19:47Z

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 · 2024-04-19T13:31:53Z

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 · 2024-04-19T13:39:48Z

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 · 2024-04-21T18:04:47Z

Exercise is reworked based (pretty closely) on your proposal.
Both introduction.md files are shortened.
Added if-then-else-expressions as a prereq.

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.

ErikSchierboom

Great work!

ErikSchierboom · 2024-04-23T06:36:13Z

exercises/concept/squeaky-clean/SqueakyClean.fs

@@ -0,0 +1,6 @@
+module SqueakyClean
+
+open System


Good idea to already have this opened!

concepts/chars/introduction.md

ErikSchierboom · 2024-04-23T06:41:44Z

concepts/chars/introduction.md

+Iterating over a string returns a `char` at each step:
+
+```fsharp
+[| for c in "F#" -> c, int c |]  //  => [|('F', 70); ('#', 35)|]
+```
+
+As shown above, a `char` can be cast to its `int` value.
+This also works (*at least some of the time*) for other scripts:
+
+```fsharp
+[| for c in "東京" -> c, int c |] //  =>  [|('東', 26481); ('京', 20140)|]
+```


Maybe instead of having a list comprehension as the example here, we use a higher-order function?

Agree. Update will follow later today.

concepts/chars/introduction.md

ErikSchierboom · 2024-04-23T06:45:21Z

exercises/concept/squeaky-clean/SqueakyCleanTests.fs

+[<Fact>]
+[<Task(1)>]
+let ``Clean_empty_string``() = clean "" |> should equal ""


I think that it would be useful for the student to have them implement the let transform (c: char) : string function first, before having to deal with the higher-order function. So we'd have the tests work on single characters first, and then later on have a couple of tests that verify that clean can work on an entire string.

This also has the benefit that it is more likely that students use the same exemplar solution, which gives great for mentoring.

Agree. Update will follow later today.

Co-authored-by: Erik Schierboom <[email protected]>

colinleach · 2024-04-23T23:34:21Z

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.

ErikSchierboom

I think this is excellent.I'm second-guessing myself a little bit on whether we want to do the full string processing at all, but it's probably fine. We can always move that bit out of this exercise and in a new exercise, for example one that has one deal with StringBuilder

exercises/concept/squeaky-clean/.docs/instructions.md

colinleach and others added 3 commits April 17, 2024 16:41

[chars and squeaky-clean] new draft concept and exercise

c38d62f

added design.md

340bc5f

expanded markdown doc

197fc2f

colinleach added 3 commits April 18, 2024 12:52

tweaks to address CI failures

8b859ed

more attempts to placate CI

9f3d441

small typos

f158b7a

ErikSchierboom reviewed Apr 19, 2024

View reviewed changes

colinleach and others added 5 commits April 19, 2024 06:09

Update concepts/chars/.meta/config.json

82289df

Co-authored-by: Erik Schierboom <[email protected]>

Update exercises/concept/squeaky-clean/.docs/introduction.md

908cb91

Co-authored-by: Erik Schierboom <[email protected]>

Update concepts/chars/introduction.md

6865bfa

Co-authored-by: Erik Schierboom <[email protected]>

Update concepts/chars/introduction.md

82f7e1f

Co-authored-by: Erik Schierboom <[email protected]>

Update config.json: formatting

0ebe6ed

revised exercise and docs

80dd9ee

colinleach added 2 commits April 21, 2024 13:48

updated hints.md

f92a497

typo

bbcf123

ErikSchierboom reviewed Apr 23, 2024

View reviewed changes

colinleach and others added 3 commits April 23, 2024 10:20

Update concepts/chars/introduction.md

9d9b589

Co-authored-by: Erik Schierboom <[email protected]>

Update concepts/chars/introduction.md

6ac58cc

Co-authored-by: Erik Schierboom <[email protected]>

Various updates to address reviewer comments

a0a98c0

ErikSchierboom approved these changes Apr 24, 2024

View reviewed changes

exercises/concept/squeaky-clean/.docs/instructions.md Outdated Show resolved Hide resolved

Update exercises/concept/squeaky-clean/.docs/instructions.md

886e559

ErikSchierboom merged commit 82afbd8 into exercism:main Apr 24, 2024
3 checks passed

colinleach deleted the squeaky-clean branch April 27, 2024 16:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

colinleach commented Apr 18, 2024

colinleach commented Apr 18, 2024 •

edited

Loading

colinleach commented Apr 18, 2024 •

edited

Loading

ErikSchierboom left a comment

ErikSchierboom Apr 19, 2024

ErikSchierboom Apr 19, 2024

ErikSchierboom Apr 19, 2024

colinleach Apr 19, 2024

ErikSchierboom Apr 19, 2024

ErikSchierboom Apr 19, 2024

colinleach Apr 19, 2024

ErikSchierboom commented Apr 19, 2024

colinleach commented Apr 19, 2024

colinleach commented Apr 19, 2024

ErikSchierboom commented Apr 19, 2024

colinleach commented Apr 21, 2024

ErikSchierboom left a comment

ErikSchierboom Apr 23, 2024

ErikSchierboom Apr 23, 2024

colinleach Apr 23, 2024

ErikSchierboom Apr 23, 2024

colinleach Apr 23, 2024

colinleach commented Apr 23, 2024

ErikSchierboom left a comment

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

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

Conversation

colinleach commented Apr 18, 2024

colinleach commented Apr 18, 2024 • edited Loading

colinleach commented Apr 18, 2024 • edited Loading

ErikSchierboom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikSchierboom commented Apr 19, 2024

colinleach commented Apr 19, 2024

colinleach commented Apr 19, 2024

ErikSchierboom commented Apr 19, 2024

colinleach commented Apr 21, 2024

ErikSchierboom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colinleach commented Apr 23, 2024

ErikSchierboom left a comment

Choose a reason for hiding this comment

colinleach commented Apr 18, 2024 •

edited

Loading

colinleach commented Apr 18, 2024 •

edited

Loading