Skip to content

Add AnyVariable as a read-only representation of Variable#697

Closed
larryonoff wants to merge 1 commit into
ReactiveX:masterfrom
larryonoff:feature/add-any-variable
Closed

Add AnyVariable as a read-only representation of Variable#697
larryonoff wants to merge 1 commit into
ReactiveX:masterfrom
larryonoff:feature/add-any-variable

Conversation

@larryonoff
Copy link
Copy Markdown
Contributor

The reason why I need this functionality:
AnyVariable can be initialized on init and is used to avoid unsafe mutability after.

Code I have right now:
Please see this PR.

PS
I'll proceed with this PR if it's accepted.
I also suggest renaming Variable to MutableVariable.

@larryonoff larryonoff changed the title add AnyVariable as a read-only representation of Variable Add AnyVariable as a read-only representation of Variable May 20, 2016
@kzaher
Copy link
Copy Markdown
Member

kzaher commented May 22, 2016

Hi @larryonoff ,

I understand why this might be convenient in some cases, and why some other libraries use similar approaches, but I wouldn't want to pull this to main repo because that would mean that we are creating parallel concept to the observable sequence that has latest value exposed.

IMHO this concept is too high level for this repo.

@larryonoff
Copy link
Copy Markdown
Contributor Author

What do you think about moving this concept into RxSwiftCommunity?

@kzaher
Copy link
Copy Markdown
Member

kzaher commented May 23, 2016

Hi @larryonoff ,

Maybe RxViewModel? It's kind of hard to me to judge where should this belong.

Could you maybe more explain some of the typical use contexts of this concept?

@larryonoff
Copy link
Copy Markdown
Contributor Author

larryonoff commented May 25, 2016

Maybe RxViewModel?

I think that it's better to do in a separate project. Because the concept isn't only for ViewModel, but can be used in other app architecture approaches.

Could you maybe more explain some of the typical use contexts of this concept?

The problem is that Variable can be exposed from business logic (e.g. ViewModel as label text), but it can be changed outside (e.g. from view) since it's mutable. So if it's unsafe exposing Variable from ViewModel.

@kzaher
Copy link
Copy Markdown
Member

kzaher commented Jun 8, 2016

The problem is that Variable can be exposed from business logic (e.g. ViewModel as label text), but it can be changed outside (e.g. from view) since it's mutable. So if it's unsafe exposing Variable from ViewModel.

This doesn't seem to me like a big difference from exposing Observable or Driver from view model.

The only difference is exposing current value, but the real question is should you ever expose it, because if you are, then you have 2 ways of fetching a value, and you need to define an sensible initial value for all values, which might not be possible to always do.

Did you maybe check out withLatestFrom operator?

@larryonoff
Copy link
Copy Markdown
Contributor Author

This doesn't seem to me like a big difference from exposing Observable or Driver from view model.

I absolutely agree and you have a very good point.

Let me rephrase my idea then. Immutable Variable can be created in ViewModel initializer (as let property) and changed only by it's Observable, as result no-one except it's Observable can change the Variable.

@kzaher
Copy link
Copy Markdown
Member

kzaher commented Jul 1, 2016

Hi @larryonoff ,

Let me rephrase my idea then. Immutable Variable can be created in ViewModel initializer (as let property) and changed only by it's Observable, as result no-one except it's Observable can change the Variable.

right, but if you have only one thing that changes a value, then you can might as well just propagate that original observable and be done with it :)

Like you've said, the difference is if you want to communicate no effects are performed. In that case specific type could be used, similar to Variable/BehaviorSubject, but without value.

I think this is similar to #771 and #731. We might want to just create a repo in RxSwiftCommunity and add these concepts into it.

@acecilia
Copy link
Copy Markdown
Contributor

acecilia commented Jul 3, 2016

Is maybe this kinda solving your problem? (just declaring the Variable class as a struct):

struct Variable2<Element> {

  public typealias E = Element

  private let _subject: BehaviorSubject<Element>
  private var _value: E

  public var value: E {
    get {
      return _value
    }
    set(newValue) {
      _value = newValue
      _subject.on(.Next(newValue))
    }
  }

  public init(_ value: Element) {
    _value = value
    _subject = BehaviorSubject(value: value)
  }

  public func asObservable() -> Observable<E> {
    return _subject
  }
}

In this way, declaring:

protocol viewModelProtocol {
  var cucu: Variable2<Int> { get }
}

class viewModel: viewModelProtocol {
  private(set) var cucu: Variable2<Int> = Variable2(4)
}

Allows you to change the Variable value from inside the viewModel but not from outside. (just an idea, could not fully test the problems of making it a struct yet due to lack of time) (maybe a mix between this and @larryonoff code is the way to go)

@larryonoff
Copy link
Copy Markdown
Contributor Author

@acecilia Thanks! But how it differs from code in my PR?

@acecilia
Copy link
Copy Markdown
Contributor

acecilia commented Jul 5, 2016

I do not quiet see how your PR works, from what I understood you still will need to declare a Variable to emit the objects that go into the observable of the AnyVariable (right?)
With the Variable as a struct you can change the value of the Variable from inside the ViewModel but not from outside, without needing to declare an extra property (as I am actually doing in #780, declaring a private Variable and an internal Observable)
Anyway, it is just an idea, as I said could not try it in code yet, quiet busy this days...

@kzaher kzaher closed this Aug 30, 2016
@kzaher kzaher mentioned this pull request May 5, 2017
3 tasks
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.

3 participants