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

BombDefuser exemplar solution and tests #496

Open
adauguet opened this issue Oct 4, 2021 · 9 comments
Open

BombDefuser exemplar solution and tests #496

adauguet opened this issue Oct 4, 2021 · 9 comments

Comments

@adauguet
Copy link
Contributor

adauguet commented Oct 4, 2021

I am trying a to mentor a student on the BombDefuser exercise.

It states:

For each bit in the ID number, starting with the leftmost bit, you will apply the flipper closure to the wires tuple if the bit is a 0 and you will apply the rotator closure if it is a 1 giving the new state of the wires.

To my understanding, this means that I should process the leftmost bit, which the left most 1 or 0.
If my number is 253 (like in the tests), its binary representation is 1111 1101 and I should process bits in the following order: 1, 1, 1, 1, 1, 1, 0 and 1.

In the exemplar solution, the implementation performs: if bits.isMultiple(of: 2). But that is checking the rightmost bit, not the leftmost.

What am I missing? Is this an error in the implementation? Or in the instructions?

Fun fact, the 113 number (used in tests) is working in both directions... leading to some confusion debugging all this.

@ErikSchierboom
Copy link
Member

What am I missing? Is this an error in the implementation? Or in the instructions?

It can be what we choose it to be. Do we want to update the instructions or the implementation? The right-to-left implementation is a bit easier I think, right? If so, it might be best to update the instructions as we don't want to have students implement a very complex solution.

Fun fact, the 113 number (used in tests) is working in both directions... leading to some confusion debugging all this.

Aha, this must be why the exemplar solution's implementation still passes the tests. We should add a new test that would fail with an invalid implementation.

Would you be willing to submit a PR to fix this?

@adauguet
Copy link
Contributor Author

adauguet commented Oct 6, 2021

The right-to-left implementation is indeed a bit easier, using % and / operators. But instructions mention bits, and they would not be used nor manipulated through bitwise operators in the exemplar solution. Which could be confusing.

In both cases, instructions should mention to encode on eight bits, just to be sure students do not forget some 0s.

Note : the String(_:radix:uppercase:) can also be useful here, and allow students to visualise bits.

I am open to submit a PR as soon as we decided where to go 🙂

@adauguet
Copy link
Contributor Author

adauguet commented Oct 6, 2021

Also, sorry for creating this issue in the wrong repo. Could you please transfer it to exercism/swift? Thanks!

@ErikSchierboom ErikSchierboom transferred this issue from exercism/swift-test-runner Oct 6, 2021
@ErikSchierboom
Copy link
Member

These are the exercise's prerequisites:

  • "functions", "higher-order-functions", "loops"

With just those prerequisites (and their transitive prerequisites), I don't think we can expect the user to know how to do bitwise manipulation. Similarly, the exercise uses the UInt8 type, which also is not known to the student. I would suggest one of the following:

Option 1:

  • Add arrays as a prerequisite
  • Change the UInt8 parameter to an array of Int values, effectively replacing the single byte with an array of 0 and 1s

Option 2:

  • Add characters as a prerequisite
  • Change the UInt8 parameter to a String that contains 0 and 1 characters.

@adauguet
Copy link
Contributor Author

adauguet commented Oct 6, 2021

You are right, bitwise operators are too complicated for a beginner. Actually there are only explained in one the last chapter of the Apple Swift book.

Are you suggesting to completely replace Int by an array Int, or a String?

@ErikSchierboom
Copy link
Member

I am.

@adauguet
Copy link
Contributor Author

adauguet commented Oct 6, 2021

OK great. Will work on the PR then.

@mkalmes
Copy link

mkalmes commented Feb 22, 2022

I've just completed the exercise myself and was confused by the description to start with the leftmost bit. Checking the bits in Calculator.app gave me the clue to my misunderstanding. If you visualize the binary representation as an array, the first bit starts at position 0 and grows to the right. However the string representation shows the Least Significant Bit (LSB) on the rightmost side.

Screen Shot 2022-02-22 at 08 40 26

@paiv mentioned the same observation in #493. I just wanted to +1 this conversation. 😄

@blynx
Copy link
Contributor

blynx commented Oct 23, 2022

I think it would be nice to treat the issue as a typo als long as the exercise isn't thorougly reworked. That would certainly prevent some confusion - as I just had, too ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants