search

exercism / x86-64-assembly

Exercism exercises in x86-64 Assembly.
https://exercism.org/tracks/x86-64-assembly
MIT License
22 stars 18 forks source link

Wrong ENUMS in Allergies #142

Open ysf opened 2 years ago

ysf commented 2 years ago

I think there is a bug in https://exercism.org/tracks/x86-64-assembly/exercises/allergies - the README states that the values defined in the enum are powers of 2, but the C file that tests against is not defining them as such. They are defined sequentially.

bergjohan commented 2 years ago

Hi,

I can see why you'd think this is a bug, however it's actually not. I'll try to explain.

The exercise it taken from https://github.com/exercism/problem-specifications/blob/main/exercises/allergies/canonical-data.json where we can see that each test case takes an item and a score as input.

The item is a string in the specification. This might be fine for languages that has good support for string handling, but for languages like C and assembly, dealing with strings is not that much fun, and we'd instead prefer to use enum values.

The values for the items in the description (https://github.com/exercism/problem-specifications/blob/main/exercises/allergies/description.md) does not correspond to the enum values, but are instead just used to test if a person is allergic to a specific item or not.

So if we pass the enum value for PEANUTS and the score of 2, we would get true since the value 2 tells us that there's a peanut allergy, and the item passed was PEANUTS.

If you think about it, it's actually a bit field, where different bits mark different allgeries.

I hope this makes sense, I would also like to add that this is how other languages do it as well, e.g. C and C#. See: https://github.com/exercism/c/blob/main/exercises/practice/allergies/allergies.h https://github.com/exercism/csharp/blob/main/exercises/practice/allergies/Allergies.cs

Let me know if anything is unclear, or if you have any further questions!

SleeplessByte commented 2 years ago

Using bitfields is exactly what I've seen in many tracks. This comment is merely to support this implementation. Powers of two encoded in a single number are (usually) bit flags in a bit field :)

ysf commented 2 years ago

Thanks for your quick reply!

I completely understand your point and know how bitfields work and what is meant to be the exercise. However, I have a mentee trying to solve this and she says that the values in the corresponding C file are not defined as specified in the readme. And I have to say that she is right. I think that either we have to adapt the tests to show that the ENUMS values are used to shift bits into place instead of ASSERTING with "magic" numbers OR adapt the readme/specification stating this. I think that everyone agrees that the exercise is not to find out what could be meant but working with bitfields in assembler instead.

bergjohan commented 2 years ago

Hm, maybe I'm wrong, the C# example solution actually uses the values in the specifications for the enum: https://github.com/exercism/csharp/blob/main/exercises/practice/allergies/.meta/Example.cs

ErikSchierboom commented 2 years ago

FYI The allergies exercise is currently causing the build to fail: https://github.com/exercism/x86-64-assembly/runs/4321821043?check_suite_focus=true