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

Converting between data and presentation types

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

Problem

Below is how I'm solving the problem of converting between data and presentation types, I'd like to know if that's a good way to go about it, and if not, what would be a better way to go about it.

I already had an IViewModel interface:

/// 
/// An interface for a ViewModel.
/// 
public interface IViewModel : INotifyPropertyChanged
{
    /// 
    /// Notifies listener that the value of the specified property has changed.
    /// 
    void NotifyPropertyChanged(Expression> property);

    /// 
    /// Notifies listener that the value of the specified property has changed.
    /// 
    void NotifyPropertyChanged(string propertyName);
}


So I added an IViewModel interface that extends it:

/// 
/// An interface for a ViewModel that encapsulates an entity type.
/// 
/// The entity interface type.
public interface IViewModel : IViewModel where T : class
{
    /// 
    /// A method that returns the encapsulated entity interface.
    /// 
    /// Returns an interface to the encapsulated entity.
    T ToEntity();
}


Then to facilitate usage, I implemented it in a base class:

```
///
/// Base class to derive ViewModel implementations that encapsulate an Entity type.
///
/// The entity type.
public abstract class ViewModelBase : IViewModel where T : class
{
protected readonly T EntityType;

///
/// Initializes a new instance of the class.
///
/// An instance of the entity type to encapsulate.
protected ViewModelBase(T entityType)
{
EntityType = entityType;
ReflectTypeProperties();
}

public T ToEntity()
{
return EntityType;
}

#region INotifyPropertyChanged implementation
///
/// Occurs when a property value changes.
///
public event PropertyChangedEventHandler PropertyChanged;

///
/// Notifies listener that the value of the specified property has changed.
///
/// The name of the property to notify about.
public void NotifyPropertyChanged

Solution

T ToEntity();


To me, ToEntity() implies some sort of conversion action. A better option might be T GetEntity() or even a property called Entity.

protected readonly T EntityType;


EntityType is a bad name for this field, because it does not contain a type, it contains the entity. Because of that, something like Entity might be better.

Also, you might want to consider making this into a property. The reasons for not using public fields also apply to protected fields (though not as strongly).

private IDictionary _propertyNotifications;


This seems completely unnecessary. Unless you know that this actually makes measurable improvement in performance (which I seriously doubt), just raise the event.

var lambda = (LambdaExpression)property;
var unaryExpression = body;


These two lines are unnecessary and I think they also don't improve readability.

memberExpression = (MemberExpression)unaryExpression.Operand;


If you're expecting only some specific UnaryExpressions, then I would check that those are actually what you have. For example, I think your code would work with () => !BoolProperty, which I think it shouldn't.

Code Snippets

T ToEntity();
protected readonly T EntityType;
private IDictionary<string, Action> _propertyNotifications;
var lambda = (LambdaExpression)property;
var unaryExpression = body;
memberExpression = (MemberExpression)unaryExpression.Operand;

Context

StackExchange Code Review Q#30769, answer score: 7

Revisions (0)

No revisions yet.