Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grade-school: align with specification per #503 #555

Merged
merged 9 commits into from
Aug 3, 2021

Conversation

wolf99
Copy link
Contributor

@wolf99 wolf99 commented Dec 29, 2020

Fixes #503

@wolf99 wolf99 requested a review from a team December 29, 2020 22:00
@wolf99
Copy link
Contributor Author

wolf99 commented Dec 29, 2020

/format

Comment on lines 55 to 70
static void test_a_student_cant_be_in_two_different_grades(void)
{
TEST_IGNORE();
roster_t input = {
2, {
(student_t) {2, "Aimee"},
(student_t) {1, "Aimee"}}
};
uint8_t desired_grade = 5;

populate_roster(&input);

roster_t actual = get_grade(desired_grade);

TEST_ASSERT_EQUAL(0, actual.count);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is really confusing. How does this ensure that a student can't be in two different grades? "Aimee" is added to grades 1 and 2, but then it checks that grade 5 is empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, copy-pasta error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this some more, the way this test is specified makes me think I should rearchitect this exercise completely.

My first inclination here was to check student names as a student is added, compare it to names already on the list and not add it if the name already exists.

However this supposes a linear timeline of added the input (i.e. add Aimee at grade 2 that succeeds, then attempt to add Aimee at grade 1 that fails). However, the desired grade is specified as 2 with an expected output of zero (implying that actually the addition of Aimee in grade 2 should have failed).

This suggests a much simpler interface like the following, where there is no local store of the roster, it is instead supplied as an input each time a query is made.

typedef struct {
   uint8_t grade;
   char *name;
} student_t;

typedef struct {
   size_t count;
   student_t students[MAX_STUDENTS];
} roster_t;

roster_t get_roster(roster_r students);

roster_t get_grade(uint8_t grade, roster_r students);

@wolf99
Copy link
Contributor Author

wolf99 commented Jan 1, 2021

I realised my idea for simplification of the interface (#555 (comment)) conflicts with the information in the exercise README.md.
I think the current interface is a bit ugly, making the tests more convoluted than they need to be; but I don't see another way to meet both the readme and the canonical-data.

So I have added a check for multiple names in the same grade in the current implementation in the latest commit instead.
Now the exercise and the implementation should match the problem specification and the readme.

@wolf99
Copy link
Contributor Author

wolf99 commented Jan 2, 2021

This still isn't right 🙃
Opening an issue on problem specifications as the spec is too conflicting

exercism/problem-specifications#1763

Base automatically changed from master to main January 28, 2021 19:15
@wolf99 wolf99 force-pushed the grade-school-update branch 2 times, most recently from 58f7525 to 00258b5 Compare February 28, 2021 21:15
@wolf99 wolf99 force-pushed the grade-school-update branch from 00258b5 to e744fdc Compare July 28, 2021 19:22
@wolf99
Copy link
Contributor Author

wolf99 commented Jul 28, 2021

/format

@wolf99
Copy link
Contributor Author

wolf99 commented Jul 28, 2021

Finally! This exercise is at a state it is possible to implement and match the spec 🙌
Ready for review.. Configlet is failing but this is for the icons for which there is #675 .

@wolf99 wolf99 requested review from ryanplusplus and a team July 28, 2021 19:26
@wolf99 wolf99 self-assigned this Jul 28, 2021
@wolf99 wolf99 added bug x:action/fix Fix an issue x:action/sync Sync content with its latest version x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/practice-exercise Work on Practice Exercises x:size/large Large amount of work labels Jul 28, 2021
@wolf99 wolf99 force-pushed the grade-school-update branch from 0233c48 to 8ba1222 Compare August 3, 2021 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug x:action/fix Fix an issue x:action/sync Sync content with its latest version x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/practice-exercise Work on Practice Exercises x:size/large Large amount of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

grade-school: update to most recent canonical specification
2 participants