Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've learned MVVM for a week or so, and I've seen the Jason Dolinger video many times. Step by step, following the Jason Dolinger video, I've created my own application which is almost the same to the one presented by Jason Dolinger. I'm completely satisfied with my current application, however, because the recorded video is several years old I think that something can probably be improved or some things are obsolete.

namespace TerminatorConsole.Model
{
    public interface IWcfModel
    {
        List<ConsoleData> DataList { get; set; }
        event Action<List<ConsoleData>> DataArrived;
    }
}

namespace TerminatorConsole.Model
{
    class WcfModel : IWcfModel
    {
        private List<ConsoleData> _dataList;

        public List<ConsoleData> DataList
        {
            get { return _dataList; }
            set { _dataList = value;
                var dataDel = DataArrived;
                if (dataDel != null)
                {
                    DataArrived(_dataList);
                }
            }
        }

        public event Action<List<ConsoleData>> DataArrived;
    }
}

namespace TerminatorConsole.Utils
{
    class DispatchingWcfModel : IWcfModel
    {

        private readonly IWcfModel _underlying;
        private readonly Dispatcher _currentDispatcher;

        public DispatchingWcfModel(IWcfModel model)
        {
            _currentDispatcher = Dispatcher.CurrentDispatcher;
            _underlying = model;
            _underlying.DataArrived += new Action<List<ConsoleData>>(_underlying_DataArrived);
        }

        private void _underlying_DataArrived(List<ConsoleData> obj)
        {
            Action dispatchAction = () =>
            {
                if (DataArrived != null)
                {
                    DataArrived(obj);
                }
            };
            _currentDispatcher.BeginInvoke(DispatcherPriority.DataBind, dispatchAction);
        }

        public List<ConsoleData> DataList
        {
            get { throw new NotImplementedException(); }
            set { throw new NotImplementedException(); }
        }

        public event Action<List<ConsoleData>> DataArrived;
    }
}

namespace TerminatorConsole.ViewModel
{
    public class OverviewViewModel
    {
        private IWcfModel model;
        public ObservableCollection<ConsoleData> DataList { get; set; }


        public OverviewViewModel(IWcfModel model)
        {
            this.model = model;
            this.DataList = new ObservableCollection<ConsoleData>();
            model.DataArrived += new Action<List<ConsoleData>>(model_DataArrived);
        }

        private void model_DataArrived(List<ConsoleData> dataList)
        {
            DataList.Clear();
            dataList.ForEach(x => DataList.Add(x));
        }

        private static ObservableCollection<T> ToObservableCollection<T>(IEnumerable<T> coll)
        {
            var c = new ObservableCollection<T>();
            foreach (var e in coll) c.Add(e);
            return c;
        }
    }

}

namespace TerminatorConsole
{
    /// <summary>
    /// Interaction logic for App.xaml
    /// </summary>
    public partial class App : Application
    {
        private WcfLoader loader;

        protected override void OnStartup(StartupEventArgs e)
        {
            base.OnStartup(e);

            IUnityContainer container = new UnityContainer();

            WcfModel model = new WcfModel();
            DispatchingWcfModel dispatching = new DispatchingWcfModel(model);
            loader = new WcfLoader(model);
            container.RegisterInstance<IWcfModel>(dispatching);

            MainWindow window = container.Resolve<MainWindow>();
            window.Show();
        }
    }
}

namespace TerminatorConsole
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {
        private OverviewViewModel _vm;

        [Dependency]
        public OverviewViewModel VM
        {
            set
            {
                _vm = value;
                this.DataContext = _vm;
            }
        }

        public MainWindow()
        {
            InitializeComponent();
        }
    }

}

namespace TerminatorConsole.Utils
{
    class WcfLoader
    {
        private IManagementConsole pipeProxy;

        Timer refresh_timer;
        private int interval = 1000;
        private WcfModel model;

        public WcfLoader(WcfModel model)
        {
            this.model = model;

            ChannelFactory<IManagementConsole> pipeFactory =
                new ChannelFactory<IManagementConsole>(
                    new NetNamedPipeBinding(),
                    new EndpointAddress("net.pipe://localhost/PipeMBClientManagementConsole"));

            pipeProxy = pipeFactory.CreateChannel();

            refresh_timer = new System.Timers.Timer(interval);
            refresh_timer.AutoReset = false;
            refresh_timer.Elapsed += OnRefreshTimedEvent;

            refresh_timer.Start();
        }

        void OnRefreshTimedEvent(object source, ElapsedEventArgs args)
        {
            try
            {
                LoadModel(model);
            }
            finally
            {
                refresh_timer.Start();
            }
        }

        public void LoadModel(WcfModel model)
        {
            var dataList = new List<ConsoleData>();
            foreach (StrategyDescriptor sd in pipeProxy.GetStrategies())
            {
                dataList.Add(pipeProxy.GetData(sd.Id));
            }
            model.DataList = dataList;
        }
    }
}
share|improve this question

1 Answer 1

There are a few things you can improve here. Unless all your classes are in separate files, you do not need to do namespace NamespaceName around each class.

This is most unnecessary:

public List<ConsoleData> DataList
{
    get { throw new NotImplementedException(); }
    set { throw new NotImplementedException(); }
}

Why define something just to make it throw an exception every time it is used? This is being abstracted from IWcfModel, so if you don't do this, it will use the DataList defined in IWcfModel. If that DataList is not correct for this instance, you should either not abstract from IWcfModel, or you should implement the correct version of this.

If you use a try block, there should usually be a catch block too:

try
{
    LoadModel(model);
}
finally
{
    refresh_timer.Start();
}

If there is no catch block and the try block throws, the finally does not run until the exception is caught. It can be caught anywhere up the call stack, but it must be caught or the application will probably crash. If LoadModel(model) does not throw, the finally will be guaranteed to run, and if it does throw and the exception is not caught, your application will probably crash. Because of this, you should either place a catch in there or the try-finally block is not needed because both statements will run in order.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.