Use expression body #3491
Use expression body #3491
Conversation
| }); | ||
| }, this.WhenAnyValue(x => x.SelectedWallet, x => x?.WalletState).Select(x => x == WalletState.Uninitialized)); | ||
| LoadCommand = ReactiveCommand.Create(() => | ||
| RxApp.MainThreadScheduler.Schedule(async () => await LoadWalletAsync()), |
There was a problem hiding this comment.
It turned out this is disposable. Can you add CompositDisposables property and add this and RootList to it and dispose the CompositDisposables in the Dispose method?
There was a problem hiding this comment.
@yahiheb this is what I meant. Every time you initialize a new object which is IDisposable you should consider adding the disposal of it. If the disposal is missing you will get a memory leak, if the disposal is there nothing happens.
Rx has a different approach to this: you only have to dispose of if you are subscribing to a non-class member object. In this case, we might not need to dispose of the IDisposable created by Schedule but I am not sure.
@danwalmsley I got some ideas after reading that. How about adding RxApp.TaskpoolScheduler to the ReactiveCommand and remove the Schedule?
There was a problem hiding this comment.
@molnard Thank you very much.
I appreciate your explanation and your help.
|
Ready to review! |
Note that I changed the LoadCommand property's type in order to use expression body:
public ReactiveCommand<Unit, Unit> LoadCommand { get; }public ReactiveCommand<Unit, IDisposable> LoadCommand { get; }Is that safe to do? I tested and everything seems to work properly.