exercism / awk

Exercism exercises in AWK.
https://exercism.org/tracks/awk
MIT License
18 stars 21 forks source link

The width of grades can exceed a single character and may even be non-numeric. #253

Open tarekmohamedibrahim opened 4 months ago

tarekmohamedibrahim commented 4 months ago

While mentored by Filbo, he revealed to me the idea of generalizing the code to accept grades with width more than one character. I checked it to accept also non-numeric grade values. I adjusted my code to pass this test, and I changed one of the testes to do this kind of test.

glennj commented 4 months ago
  1. the tests come from the exercism/problem-specifications repo: changes to existing tests should be discussed there.

  2. each exercise has an example solution to verify that the exercise can be solved: does your proposed test change pass the example solution?

  3. how does a non-numeric grade affect the sorting of grades? Does "AB" come before 1? After any numeric grade? In Ontario, we have junior kindergarten ("JK"), senior kindergarten ("SK") then grade 1. I don't know what "AB" is supposed to mean.

tarekmohamedibrahim commented 4 months ago

Hi Glenn, Here are my answers to your questions

  1. when I submitted this request, I didn't consider the example solution, but when I ran it, all tests passed, except the one test that I changed, which title is Students are sorted by grades and then by name in the roster, and if I changed the excepted result to be : Barb,Charlie,Alex,Peter,Zoe,Jim,Anna, it would work. Now, there is a problem introduced, i.e., the sort function "roster_sort", which sorts by grade, then by name, sees the grades as numerals only and it didn't consider the case of Character grades like "JK" or "SK", the function returns : v1-v2 in case of grades, this would be resolved to zero for any passed character grades, and if there are grades added in the text values as SK then JK, they would be seen as the save value when subtracted, i.e KS - SK, as they both would be resolved to zero So, and this is a question: would you like me to cancel this pull request and change the example solution to accept characters ?

  2. Based on the function "roster_sort", the "AB" comes first. Also, "AB" was supposed by me as just a character and I didn't think while suggesting it for some real grade in life, but may be there are places that use grades like "A", "B" for baby classes

glennj commented 4 months ago

We can still work in the context of this PR. Just add further commits to it.

If we're going to add non-digit grades:

I do appreciate your contribution. Please see the Practice Exercise doc for all the little details about how exercises are constructed within a track.

glennj commented 4 months ago

@IsaacG I'm OK with adding a new test. This exercise gets students to practice sorting and having non-numeric grades adds an interesting new wrinkle to it.

IsaacG commented 4 months ago

I'm not seeing a justification for these updates to be awk specific and to diverge from the problem specs.

tarekmohamedibrahim commented 4 months ago

When I initially created this PR, I was do not aware of the example solution, and I didn't make any effort to change it, which needs new/update specs. What prompted me to change the test "Students are sorted by grades and then by name in the roster", was that my initial code for this problem passed all tests, until Filbo advised me to generalize the code to accept more than one digit. I realized that all tests had only one digit in grades. My code in the last iteration accepted numerals and non-numeric values which gave me the idea to change the test to include non-numeric values. However, following Issac Good comment, I discarded the idea of introducing non-numeric grades and retained the change that used a grade with two digits ( in some countries, grade numbers before the collage are counted from 1 to 12).

tarekmohamedibrahim commented 4 months ago

However, I haven't completely abandoned my idea. I might create a new problem that involves sorting both numeric and non-numeric values.

glennj commented 4 months ago

I'm willing to go forward with this, but it has to be a new test not modifying an existing one.