sets: add sets intersection #155
Conversation
marco-m-pix4d
requested review from
odormond,
iAmoric,
SpectreVert and
aliculPix4D
sets/sets.go
Outdated
| if s.Size() < x.Size() { | ||
| for item := range s.items { | ||
| if x.Contains(item) { | ||
| result.items[item] = struct{}{} | ||
| } | ||
| } | ||
| } else { | ||
| for item := range x.items { | ||
| if s.Contains(item) { | ||
| result.items[item] = struct{}{} | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Wouldn't it be better to just swap the variables and code the loop only once?
| if s.Size() < x.Size() { | |
| for item := range s.items { | |
| if x.Contains(item) { | |
| result.items[item] = struct{}{} | |
| } | |
| } | |
| } else { | |
| for item := range x.items { | |
| if s.Contains(item) { | |
| result.items[item] = struct{}{} | |
| } | |
| } | |
| } | |
| if s.Size() > x.Size() { | |
| // Swap them to loop over the smallest set | |
| s, x = x, s | |
| } | |
| for item := range s.items { | |
| if x.Contains(item) { | |
| result.items[item] = struct{}{} | |
| } | |
| } |
There was a problem hiding this comment.
This is swapping pointers (s is a pointer):
func (s *Set[T]) Intersection(...)
but on the other hand, in Go, "assignment to the method receiver propagates only to callees but not to callers", so at the exit of the function, the contents of the set s do remain the same. Said in another way, the code proposed is safe, although confusing.
There was a problem hiding this comment.
Just wanted to notice that GitHub itself decided that "marco-m-pix4d marked this conversation resolved".
I did nothing 🙄.
I added a second commit, that I thinks makes the code less confusing.
I will use set intersection in fly_helper to determine which of the required branches are present and absent. |
Part of PCI-3790