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

FromFront and FromBack should equal if the position of the layer is logically same if we count from back or from the front of the stack #725

Open
smallB007 opened this issue Apr 1, 2023 · 6 comments
Labels

Comments

@smallB007
Copy link
Contributor

let pos = stack_view.find_layer_from_name(STACKED_SEARCH_BAR).unwrap();
let is_at_front_0 = (pos == LayerPosition::FromFront(0));
let is_at_back_1 = (pos == LayerPosition::FromBack(1));

If I have a stack that contains two items and one of the items is named "STACKED_SEARCH_BAR" which is at the top of the stack that is, it is visible and not covered by any layer, then the above should give identical answer to the comparison, i.e, FromFront(0) should be equal to FromBack(1);
But it doesn't. Tbh, I did mention this somewhere, but the introduction of very unconventional way of referring to the position of the object in a container is something else... How many times do I had to check (and still am) what Front means in that case, and ironically it means the exact opposite to what programmers are used to...

@smallB007 smallB007 added the bug label Apr 1, 2023
@gyscos
Copy link
Owner

gyscos commented Apr 3, 2023

As you can see from the definition, LayerPosition is a thin wrapper around a single number. It's an abstract way to represent things like "the 2nd from the back" or "the first from the front" independently of any container.

It's very similar to how in python the number 1 can be seen as an abstract way to get the 2nd item from any sequence, and the number -1 can be seen as an abstract way to get the last item from any sequence. Clearly 1 != -1.

Similarly, FromFront(0) and FromBack(1) happen to point to the same object in a list of 2 items, but the abstract indexes themselves are different.

If you expect equality checks to work, you need a canonical representation, not just an abstract index. You could for example only use FromBack indexes and compare that.

I suppose to help that, there could be helper methods LayerPosition::from_front(self, container_size: usize) and a similar from_back. These can help you make your positions canonical (for example only work with FromBack positions).
Or a LayerPosition::point_to_same_object(a: LayerPosition, b: LayerPosition, container_size: usize).

Note that StackView::find_layer_from_name will only return FromBack indexes, so you can already compare different values returned from that function.

@smallB007
Copy link
Contributor Author

smallB007 commented Apr 3, 2023

I don't think I've expressed myself clearly. In stackview containing two items, where the "looked for" item is the item that is visible, should this piece of code compare equal:

let pos = stack_view.find_layer_from_name(STACKED_SEARCH_BAR).unwrap();
let is_at_front_0 = (pos == LayerPosition::FromFront(0));
let is_at_back_1 = (pos == LayerPosition::FromBack(1));

that is, is_at_front_0 should equal true, as well as is_at_back_1 should equal true. Are you agreeing with me on that?

@gyscos
Copy link
Owner

gyscos commented Apr 3, 2023

No: pos here is of type LayerPosition. This type is either FromFront or FromBack: you can check the type definition, it's just a regular enum.

It also does not know about the length of the StackView at this point. So it could not equal both FromFront(0) and FromBack(1) - doing so would require knowing that there's 2 elements in the StackView.

If you want to compare two positions, you could write this function:

fn point_to_same_object(pos_a: LayerPosition, pos_b: LayerPosition, container_size: usize) -> bool {
   match (pos_a, pos_b) {
        (LayerPosition::FromFront(a), LayerPosition::FromFront(b)) | (LayerPosition::FromBack(a), LayerPosition::FromBack(b)) => a == b,
        (LayerPosition::FromFront(a), LayerPosition::FromBack(b)) | (LayerPosition::FromBack(a), LayerPosition::FromFront(b)) => a + b + 1 == container_size
    }
}

Then you could check let is_at_front = point_to_same_object(pos, LayerPosition::FromFront(0), stack_view.len());.

@smallB007
Copy link
Contributor Author

smallB007 commented Apr 3, 2023

"No: pos here is of type LayerPosition. This type is either FromFront or FromBack: you can check the type definition, it's just a regular enum."
Hehe, you've created a monster. Thanks, that sure clears things for me. I hope it also highlights for you the problem with such approach to API.
Logically, if stack has two items, from_front(0) and from_back(1) should point to the same item. Anyway, that particular API should be removed and replaced with standard approach to describing position of an item in a container.

@gyscos
Copy link
Owner

gyscos commented Apr 3, 2023

They do point to the same item, but that was not your question.

@smallB007
Copy link
Contributor Author

Yes it was. Again, perhaps I didn't somehow make it clear, but I just wonder how clearer can I say it:
"then the above should give identical answer to the comparison" << meaning, if they do point to the same item, then the comparison should give identical answer to pos == FromBack(1) and pos == FromFront(0).

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

No branches or pull requests

2 participants