sets: add the Add() method #156
Conversation
marco-m-pix4d
requested review from
odormond,
iAmoric,
SpectreVert and
aliculPix4D
| @@ -60,6 +60,15 @@ func (s *Set[T]) Contains(item T) bool { | |||
| return found | |||
| } | |||
|
|
|||
| // Add inserts item into s. Returns true if the item was present. | |||
| func (s *Set[T]) Add(item T) bool { | |||
There was a problem hiding this comment.
exactly what I was missing..
I had to collect all the branches in a slice and then create a set from the slice...
I think with Add this will be much faster (computation wise...), unless really all branches are unique and slice of branches is basically a set...
There was a problem hiding this comment.
Yes, when I realized that Add() was missing, I realized that SkyGuide must be collecting a list and calling sets.From(), which doesn't make sense I think.
There was a problem hiding this comment.
Its not yet used in SkyGuide (I think?). I am using Skyguide Concourse pkg in my custom tool to inspect pipelines. There I required set, and there I was using sets.From() sadly...
|
Also, I think we should start thinking about moving some of this small pkgs to its own repo away from cogito.. e.g. sets, retry etc... It makes little sense to "depend" (not really but...) on cogito release |
Ah, I am tempted to agree with this. I was already confused in the last PR. « ok we use this in fly_helper but why it’s in Cogito ? 🤔 » |
|
To be 100% clear: we do not depend on a cogito release; we simply depend on a I agree that it seems that we are almost ready to take the utility functions into their own (public) repo; when we begun, we didn't know how much reuse we would have had... |
Part of PCI-3790