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

Convert free functions to static functions #2926

Closed
mdiep opened this issue May 23, 2016 · 5 comments
Closed

Convert free functions to static functions #2926

mdiep opened this issue May 23, 2016 · 5 comments
Milestone

Comments

@mdiep
Copy link
Contributor

mdiep commented May 23, 2016

Should non-instance methods be free functions or static functions?

Because currently we have:

extension SignalType {

    /// Merges the given signals into a single `Signal` that will emit all values
    /// from each of them, and complete when all of them have completed.
    @warn_unused_result(message="Did you forget to call `observe` on the signal?")
    public static func merge<S : SequenceType where S.Generator.Element == Signal<Value, Error>>(signals: S) -> ReactiveCocoa.Signal<Self.Value, Self.Error>
}

But we also have:

/// Combines the values of all the given signals, in the manner described by
/// `combineLatestWith`.
@warn_unused_result(message="Did you forget to call `observe` on the signal?")
public func combineLatest<A, B, Error>(a: Signal<A, Error>, _ b: Signal<B, Error>) -> Signal.Signal<(A, B), Error>

We should standardize on one or the other for RAC 5.0.

I'd vote free functions, but I'm curious if someone thinks we should be using static functions. @ReactiveCocoa/reactivecocoa

@mdiep mdiep added this to the 5.0 milestone May 23, 2016
@JaviSoto
Copy link
Member

Static functions are more autocomplete-friendly, so I would vote for that.
Thanks for bringing this up :) definitely agree we should unify for consistency!

@mdiep
Copy link
Contributor Author

mdiep commented May 23, 2016

A benefit of free functions is that you don't need to think about whether something is a Signal or a SignalProducer.

@RuiAAPeres
Copy link
Member

I agree with @JaviSoto. For people that know RAC well, free functions are not an issue, I would argue for novice to mid level discoverability can be an issue.

@andymatuschak
Copy link
Contributor

The Swift API guidelines are pretty clear on this.

Prefer methods and properties to free functions. Free functions are used only in special cases:

When there’s no obvious self:
min(x, y, z)

When the function is an unconstrained generic:
print(x)

When function syntax is part of the established domain notation:
sin(x)

These functions are basically fancy constructors, so using the metatype as self seems plenty defensible.

@mdiep mdiep changed the title Static functions or free functions? Convert free functions to static functions Jun 1, 2016
@mdiep mdiep removed the proposal label Jun 7, 2016
This was referenced Jun 29, 2016
@mdiep
Copy link
Contributor Author

mdiep commented Jul 23, 2016

This was done in #3001! 🎉

@mdiep mdiep closed this as completed Jul 23, 2016
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

4 participants