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

and() logical operator returns leftover number items when arrays are passed #8

Open
efeichen opened this issue Jun 20, 2018 · 6 comments

Comments

@efeichen
Copy link
Contributor

So I was fixing the "bug" with binaryOperation(), and made changes so that new and old tests passed, except one:
screen shot 2018-06-20 at 4 50 35 pm
This doesn't really make sense to me because and() and the contains() I'm implementing now are similar in that they both return booleans. We already established that the contains() function should only return booleans and instead of adding unprocessed array items into the return array. Yet here, the expected result of the and() does the exact thing that we decided is not right.

Should this be fixed or is there a difference between and() and contains() in terms of what they should return that I'm missing here?

@LeaVerou
Copy link
Member

Oh interesting! The previous behavior seems definitely suboptimal.
However, I'm also not sure about extra items returning true. It seems less surprising to me if (empty) and something returns false.
@karger, any thoughts?

@karger
Copy link

karger commented Jun 20, 2018

I think we were discussing this a couple days ago. If I remember right, you suggested that when one list is shorter it should be extended with the operator's identity element---which in this case would be true, so returnins false, false, 2, 3 would be correct.

@efeichen
Copy link
Contributor Author

ya, I agree that maybe we should change the identity to false or something.

(also it's weird that I'm typing a response to you rather than talking to you who was right behind me. I guess I'm preparing for your leave in July, LOL)

@karger
Copy link

karger commented Jun 20, 2018

the identity for and has to be true, because that's the only thing that has no effect when combined with other args.

@LeaVerou
Copy link
Member

I think we were discussing this a couple days ago. If I remember right, you suggested that when one list is shorter it should be extended with the operator's identity element---which in this case would be true, so returnins false, false, 2, 3 would be correct.

Does it make sense to you though?

(also it's weird that I'm typing a response to you rather than talking to you who was right behind me. I guess I'm preparing for your leave in July, LOL)

Not weird. This is asynchronous, which means I can choose to see it when I take a break and/or am less focused, so it's preferable for things that can wait (i.e. if you’re not stuck).

@efeichen
Copy link
Contributor Author

efeichen commented Jun 21, 2018

Wait, extending with identity meaning that the shorter array, in that example is [true,false] would technically be compared with the first array as [true,false,true,true]? If so, after the two are compared element-wise, shouldn’t the final result be [false,false,true,true] like the next test?

Also, the order of the array parameters should not affect the result of the operation right?

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

3 participants