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

cleanup / #43 | Deprecated unit #53

Merged
merged 1 commit into from Sep 19, 2016
Merged

Conversation

nikita-leonov
Copy link
Collaborator

@nikita-leonov nikita-leonov commented Sep 18, 2016

As suggested in #43 unit deprecated in honor of Optional.Some. Documentation removed. Unit-test removed, to avoid deprecation warning during compilation. I believe it is legit, as there are no guarantees anymore on the workability of deprecated functionality, as well as no plans to support it in future.
unit should be removed completely in the future major release.

@335g
Copy link
Collaborator

335g commented Sep 19, 2016

Looks good to me ✨

I have only one concern. Compiler doesn't infer type to use .some alone.

let a = unit(2) // OK type inference
let b = .some(2) // Error type inference
let c = Optional<Int>.some(2) // OK type inference

Compiler infer type to use as an argument. 😅

func sumOne(x: Int?) -> Int? {
    guard let x = x else {
        return nil
    }

    return x + 1
}

sumOne(x: .some(1)) // OK type inference
sumOne(x: unit(1)) // OK type inference

@335g
Copy link
Collaborator

335g commented Sep 19, 2016

unit should be removed completely in the future major release.

I think so too in the future. 👍

@nikita-leonov
Copy link
Collaborator Author

Thanks!

The compiler handles the work of type inference each time when an amount of data in context is enough. If it does not, it is not Prelude issue, but a compiler issue that needs to be reported. In a first example let b = .some(2) will never work since it should be Optional.some(1) and type of optional will be inferred.

Most of the time unit used not to initialize values, but to prepare functions for combining, something like values.map(Optional.some), in such case type inference is unavoidable.

Also in a Swift 2.3 unit have almost no value, as now even the following definition is fully legit and does not require any mapping:

let values: [Int] = [1, 2, 3]
let optionalValues: [Int?] = values

@nikita-leonov
Copy link
Collaborator Author

@robrix I guess we still need your blessing to merge, even that we theoretically have a button to do so.

@robrix
Copy link
Owner

robrix commented Sep 19, 2016

I think this is a good change. Thank you!

@robrix robrix merged commit 5850b80 into robrix:master Sep 19, 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

Successfully merging this pull request may close these issues.

None yet

3 participants