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

Swift 3.0 compatibility #22

Open
shergin opened this issue Aug 23, 2016 · 8 comments
Open

Swift 3.0 compatibility #22

shergin opened this issue Aug 23, 2016 · 8 comments
Assignees

Comments

@shergin
Copy link

shergin commented Aug 23, 2016

How about Swift 3.0 compatible version or branch?

@evgeniyd
Copy link

For reference, this fork is an attempt to migrate the project to Swift 3

@yankodimitrov yankodimitrov self-assigned this Feb 8, 2017
@yankodimitrov
Copy link
Owner

I will work on it in the coming weeks.

I would like to remove some unnecessary parts like the CollectionEvent , UITableView and UICollectionView bindings.

Also the versioning came out of control 😅 if I introduce the API breaking changes by following the semantic versioning the next version should be 5.0. What do you think should I just bump the version to 4.1?

If you have any suggestions, please share them here.

Much appreciated!

@shergin
Copy link
Author

shergin commented Feb 8, 2017

Yay!!1
I believe there is noting bad in bumping major version!

@yankodimitrov
Copy link
Owner

yankodimitrov commented Mar 5, 2017

Im working on it on the SignalKit-5.0 branch.

I refactored the Signal and SignalValue to be simple non-thread safe classes by default.
Usually those are intended to create a binding between ViewModel and the View on the main thread.
If the ViewModel is doing some async work on a background thread it will have the option to send the result to the Signal on the main thread.

Of course we can create a thread safe Signal and SignalValue by passing a Lock protocol implementation (there is a default public MutexLock implementation) or we can use the class factory method .atomic() and .atomic(value: T):

// non-thread safe
let age = Signal<Int>()
let name = Signal<String>(value: "John")

// thread safe
let age = Signal<Int>(lock: MutexLock())
let name = SignalValue<String>(value: "John", lock: MutexLock())

// thread safe
let age = Signal<Int>.atomic()
let name = SignalValue<String>.atomic(value: "John")

Hope to hear your feedback 😃

@shergin
Copy link
Author

shergin commented Mar 6, 2017

Yay! 🎉
I always prefer having default implementation simple as possible, so I think this is the right move!
I also slightly prefer to have Signal<Int>(atomic: true) instead of Signal<Int>.atomic()... I believe that it is more Swifty style.
But, feel free to ignore my advices because... I don't know the difference between Signal and SignalValue. 😄

Awesome work, Yanko! Thank you!

@yankodimitrov
Copy link
Owner

You are right its more cleaner as an initializer parameter, so I refactored it to:

let name = Signal<String>(atomic: true)

Probably because SignalValue is a bad name so I renamed it to SignalProperty. 😅
The Idea is that SignalProperty have an initial/current value and sends it to new observers right away, so you don't have to send an initial value as with the basic Signal:

// with SignalProperty
let name = SignalProperty<String>(value: "John", atomic: true)
name.bindTo(textIn: nameLabel).disposeWith(bag) // nameLabel.text is now "John"

// with Signal
let name = Signal<String>(atomic: true)
name.bindTo(textIn: nameLabel).disposeWith(bag)
name.send("John") // we have to send the initial value after we have signal observers

Also I added macOS target 🎉

Thanks!

@shergin
Copy link
Author

shergin commented Mar 7, 2017

Wow! That is great that you designate this functionality (SignalProperty) as a first class citizen of SignalKit! Really, these are different and equally important concepts and we have to have them both!

Personally, I slightly prefer to name SignalProperty as something that extends Signal concept, so it should have a name pattern <Adjective>+"Signal" like... PersistentSignal. But, again, as an author, you have to know better. 😄

@yankodimitrov
Copy link
Owner

I like the idea.

Here are some explorations for a signal that reacts to new observers by sending them its current value:

  • ReactiveSignal
  • StoredSignal

StoredSignal sounds good, but yes naming things is hard 😄

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

3 participants