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

Allergies READ ME needs clarification #224

Closed
rebelwarrior opened this issue Apr 10, 2016 · 19 comments
Closed

Allergies READ ME needs clarification #224

rebelwarrior opened this issue Apr 10, 2016 · 19 comments

Comments

@rebelwarrior
Copy link

Hi There are three ways to interpret the Allergies problem with regards to large scores. Below are the options. The first one passes the test in Elm-lang but seems incorrect to me. The second seems correct but fails the tests. And the third is possible but tests should catch it better.

If the score is >= 256 then

  1. Subtract 256 from the score and calculate.
  2. Ignore numbers above 256; treat score as 255 and calculate.
  3. Subtract 128 and mark the highest allergy and calculate, if the score is still above 128 subtract it again and calculate. You might end up with an allergy duplicated or just counted twice.

So following the first case a score of 257 equals a score of 1 since 257 - 256 is 1. This doesn't seem right to me, but if it is right then it should be stated in the Read Me.

The second is the one the READ ME alludes to but the tests fail.

Which is the way this should go? Should we clarify the READ ME with:

Scores are a representation of binary and cycle, so a score of 256 is the same as 0 and a score of 257 as 1.

Or do we need to updated the tests and the READ ME with:

Scores higher than the maximum score should be treated as the maximum score.

@wobh
Copy link

wobh commented Apr 10, 2016

It's not cycle but ceiling. I think scores higher than 255 should be treated as invalid inputs--the system can't interpret those as allergies. Cycling means making a presumption about all larger values. If you add allergy to a cyclical system you will have invalidated all previous interpretations of values greater than the max.

@kytrinyx
Copy link
Member

If you interpret these as bitmasks, then it doesn't matter that the number is higher than 256. Either the bit is flipped or it's not.

@wobh
Copy link

wobh commented Apr 10, 2016

Also ceiling is also a presumption: 256 == 255.

I'm on the fence about "fixing" this design. An advanced exercise could be dealing with the fallout of the current implementation, a body of accumulated data, and the requirement of adding additional allergies.

@kytrinyx
Copy link
Member

Right 0-255 would be the valid range.

@wobh
Copy link

wobh commented Apr 10, 2016

The mask could be seen as the floor problem: 256 == 0. That would be problematic in the same way. Sometimes you just need nil. 😀

I think of a school importing transfer students data, whose allergy scores are based on a different system. If they can't translate a given score, they probably shouldn't enter apparently valid score from the system they have.

@rebelwarrior
Copy link
Author

The cyclical solution is the one that passes the tests in Elm.

I'd rather treat values larger than 255 as invalid and error out or return nil or Nothing (in the case of Elm) if that's the way you're leaning. But this will fail current tests at least in Elm.

Sent from my mobile.

On Apr 10, 2016, at 2:58 PM, William Clifford notifications@github.com wrote:

It's not cycle but ceiling. I think scores higher than 255 should be treated as invalid inputs--the system can't interpret those as allergies. Cycling means making a presumption about all larger values. If you add allergy to a cyclical system you will have invalidated all previous interpretations of values greater than the max.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@kytrinyx
Copy link
Member

If they can't translate a given score, they probably shouldn't enter apparently valid score from the system they have.

That's a good point.

Once we decide on an approach or a fix, we should:

@IanWhitney
Copy link
Contributor

An interesting perspective was brought up by one of the participants in my Kata group. The student has food allergies, and he values false positives more than false negatives (i.e., he'd rather be told, incorrectly, that he's allergic to tomatoes than be told, incorrectly, that he's not).

So his implementation of this exercise was to default everything to true. If the system got invalid input, it would say that the patient was allergic to everything.

@kytrinyx
Copy link
Member

That's really, really interesting—and could save lives.

@wobh
Copy link

wobh commented May 28, 2016

If the system gains a reputation for false positives, it stops being trusted and stops being used.

@IanWhitney
Copy link
Contributor

I think I've pushed us from discussing the problem as a learning exercise and into discussing a real world implementation. My fault.

The problem, as I understand it, is what to do with values greater than 255, right?

I think it depends on what you're trying to teach.

If the problem is meant to teach about bitwise operators (and nearly every solution I've seen uses bitwise operators), then you should have a test that shows off the benefits of bitwise. Either testing the score 257 or 509 work for me, but I don't think you need both.

If you want to focus the problem on handling correct values, then have a test that results in errors for values outside of 0..255.

@kytrinyx
Copy link
Member

The thing about these higher numbers, though, is that all of the defined bits are still valid, it's just that we don't have any definitions for the higher-value bits. So the input isn't garbage, per se. I recently worked on a configuration/subscription system that used a bitmask and we didn't care what the input value was, only whether the bits we cared about were flipped or not.

That said, I'm fine with changing this to be errors outside of the defined range.

@IanWhitney
Copy link
Contributor

I think expecting (and testing for) normal bitwise implementations makes sense. So, either a test for 257 or 509 (or whatever). But only one, since that's enough to cover the thing that you're trying to test.

@kytrinyx
Copy link
Member

Agreed, one should be enough.

@ErikSchierboom
Copy link
Member

I'm also strongly in favor of the "bitwise" interpretation. The actual value of the number passed in does not matter, as long as the correct bits are set. I think 257 looks it bit too much like a boundary value test, while 509 might make it more apparent that you don't actually care what the number is.

@rbasso
Copy link
Contributor

rbasso commented Sep 7, 2016

I'm also strongly in favor of the "bitwise" interpretation.

Agreed, but restricted to positive numbers, of course! 😄

@kytrinyx
Copy link
Member

kytrinyx commented Sep 9, 2016

Agreed, but restricted to positive numbers, of course!

Haha, yes, absolutely!

@rbasso
Copy link
Contributor

rbasso commented Oct 3, 2016

Any reason to keep this still open after #389 was merged?

@petertseng
Copy link
Member

I think it is safe to close.

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

7 participants