RxSwiftExt
RxSwiftExt copied to clipboard
RxSwiftCommunity
[Vote] Naming for Operators that target Collections/Arrays
We're trying to figure out a good naming scheme for operators that target a collection of something.
For example, like our existing mapMany operator. One of the newer ones is bind(to: [..]), vs. bindMany(...).
The big question is, should we keep on doing *many, or should we use overloads. The cost of overloads mainly comes down to performance, indexing, collision-issues, etc.
Please cast your vote!
I personally feel that bind(to:) and drive(_) have way too many overloads already, which unnecessarily slows down compilation times. I'd also rather go with something like mapAll(_), bindAll(_), etc.
We already have mapMany, so that might need a rename if All would be preferred. I'll do a consecutive poll after this one is done :) Thanks for the feedback!
One thing that would have been very nice is that bind(to: ...), drive(...) etc be made to accept a variable number of observers. I assume the conflicts with the base implementation will make it impractical, but still the idea to have the freedom specifying any number of observers is enticing.
This said, and regardless the performance issues with tooling which may resolve themselves in a decade or two, bind(all: ...), drive(all: ...) are closer to the current Swift naming conventions.
My second choice is what @icanzilb mentioned: bindAll, driveAll, mapAll.
@fpillet To be honest that PR (#166) defines a variadic companion. This might be a good thing to try to pursue on the main repo, possibly.
We can do it in a relatively backwards compatible way, like this:
public func bind<O: ObserverType>(to observers: O...) -> Disposable where O.E == E {
return self.bind(to: observers)
}
public func bind<O: ObserverType>(to observers: [O]) -> Disposable where Self.E == O.E {
switch observers.count {
case 1:
return self.subscribe(observers[0])
default:
let shared = self.share()
let disposables = observers.map(shared.subscribe)
return CompositeDisposable(disposables: disposables)
}
}
Think this could possibly be a good idea for a PR on the main repo. What do you think? @fpillet
Opened a basic PR for discussion here: https://github.com/ReactiveX/RxSwift/pull/1702 cc/ @sgtsquiggs
We can add it but it doesn’t really make sense / is aligned with swift naming convention IMO
drive(in ... ) makes sense because you a literally driving into a container of things. It's like bind(to: ...) IMHO
If that was the case, the original operator would be called drive(in:), like bind(to:) applies to singular elements.
I don't know any place in stdlib or Swift in general where the in: notation is used to describe a collection. It's not "a container of things", it's just a collection things.
If have to stick strictly to swift convention we wouldn't invent anything. IMO drive(in: ...) stands out especially for code completion with a clear intent because only drive(...) is already fully overloaded.
Yes but when it comes to naming convention it needs to convey clarity, intent, and known practices of Swift.
in: usually refers to references, or when you “look something up”, but not to describe applying something to many elements IMO.
The only thing is it doesn’t sound like a proper English sentence, grammatically speaking. Which is what we should strive for.
Bind to (an) array sounds fine Drive to (an) array sounds wrong Drive (an) array sounds fine, which is why I think it’s worth the overload for discoverability.
In any sense, if Krunoslav would agree having the variadic option it would solve overloads since there would be a single option.
Ok then we should speak proper English then at this point. How about drive(into array: [T]) or drive(into collection: ...)
The problem with into is that in Swift it’s usually used for “In place mutation” (see reduce(into:) )
Not meaning to be anti your suggestion, but finding a good option for drive that makes good grammatical and syntactical sense would be quite hard.
@freak4pc not at all we're just brainstorming on names. I've exhausted my names.. but drive(onto collection: ... ) or drive(into collection: ...) could still be ok. I know reduce is a combination, but at the same time it works with Sequence ... Boh 🤷🏽♂️ naming is HARD! 🔨
I'd like to raise the precedent that this repo set by changing map(to:) to mapTo(). In #117, @fpillet said
So this means that we will roll back to mapTo since it is not acceptable that we shadow a core operator.
I’m aware of that precedent. Overloading works much better but is still the least preferable way to go, IMO. Unless it would be in high demand, which it doesn’t seem to be at this point.