CODEWITHSUNDEEP
c#wpfmvvm
poco
poco

Reputation: 3005

MVVM am i implementing this correctly

I have been trying to wrap my head around mvvm for the last week or more and still struggling a bit. I have watched Jason Dolingers MVVM video and gone through Reed Copsey lessons and still find myself wondering if i am doing this right... i created a very simple clock application which i will post below. The output of the program is as expected however I'm more interested in if I'm actually using the pattern correctly. Any thoughts comments etc would be appreciated.

my model

using System;
using System.Threading;

namespace Clock
{
    public class ClockModel
    {
        private const int TIMER_INTERVAL = 50;

        private DateTime _time;

        public event Action<DateTime> TimeArrived;

        public ClockModel()
        {
            Thread thread = new Thread(new ThreadStart(GenerateTimes));
            thread.IsBackground = true;
            thread.Priority = ThreadPriority.Normal;
            thread.Start();
        }

        public DateTime DateTime
        {
            get
            {
                return _time;
            }
            set
            {
                this._time = value;
                if (TimeArrived != null)
                {
                    TimeArrived(DateTime);
                }
            }
        }

        private void GenerateTimes()
        {
            while (true)
            {
                DateTime = DateTime.Now;
                Thread.Sleep(TIMER_INTERVAL);
            }
        }
    }
}

my view

<Window x:Class="Clock.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:ViewModels="clr-namespace:Clock"
        Title="MainWindow" Height="75" Width="375">
    <Window.DataContext>
        <ViewModels:ClockViewModel />
    </Window.DataContext>
    <StackPanel Background="Black">
            <TextBlock Text="{Binding Path=DateTime}" Foreground="White" Background="Black" FontSize="30" TextAlignment="Center" />
    </StackPanel>
</Window>

my view model 

using System;
using System.ComponentModel;

namespace Clock
{
    public class ClockViewModel : INotifyPropertyChanged
    {
        private DateTime _time;
        private ClockModel clock;

        public ClockViewModel()
        {
            clock = new ClockModel();
            clock.TimeArrived += new Action<DateTime>(clock_TimeArrived);
        }

        private void clock_TimeArrived(DateTime time)
        {
            DateTime = time;
            this.RaisePropertyChanged("DateTime");
        }

        public DateTime DateTime 
        {
            get
            {
                return _time;
            }

            set
            {
                _time = value;
            }
        }


        /// <summary>
        /// Occurs when a property value changes.
        /// </summary>
        public event PropertyChangedEventHandler PropertyChanged;

        /// <summary>
        /// Raises the property changed event.
        /// </summary>
        /// <param name="propertyName">Name of the property.</param>
        private void RaisePropertyChanged(string property)
        {
            if (PropertyChanged != null)
            {
                PropertyChanged(this, new PropertyChangedEventArgs(property));
            }
        }
    }
}

Upvotes: 1

Views: 624

Answers (3)

Nano Taboada
Nano Taboada

Reputation: 4182

In my opinion your implementation looks good in terms of separation of concerns although you might be interested in delegating your Model method to a Command then you could attach it to let's say the Loaded event of your main UI. It's definitely a personal preference but as good practice I tend to try to keep a 1:1 relationship between View:ViewModel and Model.Method:Command

Upvotes: 1

Johnny
Johnny

Reputation: 11

For some normal features, using MVVM is quite easily, when you start to touch showing message box. display separate windows, and communication between view and viewmodel. then you will find something tricky..

Upvotes: 0

Thomas Levesque
Thomas Levesque

Reputation: 292765

The way you're doing it is fine. There's just one thing I would change : move the call to RaisePropertyChange to the setter of the property. That how it's usually done, and it prevents you from forgetting to raise the notification when you set the property.

Upvotes: 2

Related Questions