RehanSaeed / rehansaeed.github.io

Muhammad Rehan Saeed's Blog
https://rehansaeed.com
30 stars 6 forks source link

[Comment] Model-View-ViewModel (MVVM) - Part 2 - IDisposable #91

Open RehanSaeed opened 4 years ago

RehanSaeed commented 4 years ago

https://rehansaeed.com/model-view-viewmodel-mvvm-part2-idisposable/

RehanSaeed commented 4 years ago

Muhammad Rehan Saeed Muhammad Rehan Saeed commented on 2014-09-10 08:43:45

I had a very interesting conversation with 'holymoo' on Reddit that I thought I'd post here.

@holymoo So, the value with implementing this IDisposable base class is to more effectively manage memory when it comes to working with hardware components like the gps and the compass. Now I completely agree that these object need to be disposed of when their usefulness ends. I think a base class like this adds a lot of overhead for doing an operation that can be accomplished by simply using a using statement or calling dispose. Maybe another way to phrase this would be, when is there a situation where you would want the viewmodel to implement IDisposible?

@RehanSaeed Sometimes, you want disposables to exist for the lifetime of the view model. Not just things like GPS and compass but event subscriptions which should be disposed unless you like memory leaks and other things which are more likely to popup if you use WPF with COM components for example. Starting with a disposable base class and building on top of that gives this ability if its needed. Its totally optional to dispose something or not. As for performance, it has almost zero impact. Adding an extra class in the inheritance hierarchy and doing a few boolean checks has a negligible performance impact. I have thought if its worth it in some of my simpler WinRT projects as you have. If you don't need it, its pretty simple to remove or you can ignore it.

@holymoo I can see that. I typically try to avoid writing classes which inherit from IDisposable for the sake of trying to architect things in such a way to keep using statements as small as possible.

@RehanSaeed Correct. But even Microsoft does it. They implement IDisposable and then tell you not to use it. HttpClient is a good example.

RehanSaeed commented 4 years ago

Thomas Thomas commented on 2017-01-11 16:43:03

Where are you getting the Unit ans Observable types?

RehanSaeed commented 4 years ago

Muhammad Rehan Saeed Muhammad Rehan Saeed commented on 2017-01-12 10:49:14

Where are you getting the Unit ans Observable types?

From the Reactive Extensions NuGet packages.

tchoha commented 1 year ago

I found your article quite helpful. However I'm considering what will happen if Dispose is called concurrently. Maybe a guard with Interlocked.Exchange is needed to address such cases. Something like:

int _IsDisposed;
public bool IsDisposed => _IsDisposed != 0;
private void Dispose(bool disposing)
{
    bool alreadyDisposed = Interlocked.Exchange(ref _IsDisposed, 1) != 0;
    if (!alreadyDisposed)
    {
        ...

Please let me know your opinion on that matter?

RehanSaeed commented 1 year ago

@tchoha Yes, you have the correct answer if you need to add thread safety for some reason. More info below:

https://stackoverflow.com/questions/8927878/what-is-the-correct-way-of-adding-thread-safety-to-an-idisposable-object