HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

MVVM implementation based on Jason Dolinger's video

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
jasonmvvmvideobasedimplementationdolinger

Problem

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 DataList { get; set; }
event Action> DataArrived;
}
}

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

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

public event Action> 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>(_underlying_DataArrived);
}

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

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

publi

Solution

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 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.

Context

StackExchange Code Review Q#6183, answer score: 2

Revisions (0)

No revisions yet.