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

WPF Window with different DataTemplates

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

Problem

I have to do a task hard for me and maybe you can help me. I've tried to design a window dialog to update data from a database view. This database view contains integer, DateTime, string, Time, Boolean properties.

First of all, I load the database view data into a GridControl. The GridControl is readonly not editable. I configure the columns of my GridControl dynamically and this configuration is located in a database table called FieldDescription.

When the end-user doubleclicks on a cell, this window dialog pops up. This dialog shows a Combobox when the type of the column is enum or bit (boolean) and otherwise a TextEdit as default content.Notice for example that I know that a GridColumn represents an enum type because of the FieldDescription. Because the type of this database view property bound to this GridColumn is an integer (specifically the value)

The code of this window dialog is (I use DevExpress controls):
























































I have only one ViewModel to provide the values to this Dialog Window. My ViewModel has an enum FieldDataType property called TargetFieldType. It contains the type of the doubleclicked GridColumn. This value is an enum (FieldDataType.DatTime, FieldDataType.Time, FieldDataType.Date, FieldDataType.Bit, FieldDataType.Num, FieldDataType.Enum, FieldDataType.String).

Solution

ViewModel

IMHO #region should be banned; if you need to split your class in regions, it's probably doing too many things. If you need to split a method in regions, it's definitely doing too many things.

Maybe it's just me, but I like seeing the class' constructor as the first thing in the class - with the only possible exception of some private readonly fields, constructor-injected.

Your #region properties includes private fields. I would simply put the backing field close to the property it's backing:

private object _targetValue;

/// 
/// Contains the new value to be saved in database
/// 
public object TargetValue
{
    get { return _targetValue; }
    set
    {
        _targetValue = value;
        OnPropertyChanged();
    }
}


I've never used NotificationObject, but from what I've just read on MSDN it's implementing INotifyPropertyChanged, and by not specifying which property has changed I believe you are invalidating your entire ViewModel, which I find is inefficient - the end result might be the same, but depending on how complex your View is this could mean lots of redundant/useless updates. If TargetValue was modified, notify a change on the TargetValue property.

I think your Action delegates shouldn't start with "do" - they're actions, of course they do something! Given you called them DoOnOk and DoOnCancel, I'd rename them OnOkAction and OnCancelAction.

Your DbvBigViewUtils has a bad smell. It's a static ambient context dependency that's tightly coupled with your ViewModel, which means if you were to try and write unit tests for your class, you would have no way of mocking that dependency and fully controlling your class' behavior.

Code Snippets

private object _targetValue;

/// <summary>
/// Contains the new value to be saved in database
/// </summary>
public object TargetValue
{
    get { return _targetValue; }
    set
    {
        _targetValue = value;
        OnPropertyChanged();
    }
}

Context

StackExchange Code Review Q#36256, answer score: 2

Revisions (0)

No revisions yet.