WPF Binding to a Repository with Entity-Framework

Question

I have an MVVM WPF application that will make use of a repository and unit-of-work.

This is my Entity Framework database-first model:

public partial class tUser
{ 
 public string ID { get; set; }
 public string Name { get; set; }
 public string Surname { get; set; }
 public string Password { get; set; }
}

My implementation of the UsersRepository - noticed I didn't use a Generic Repository pattern:

public class UsersRepository : IUserRepository
{
 private DbEntities db;
 public UsersRepository (DbEntities db)
 {
 this.db = db;
 }
 public tUser GetById(object userId)
 {
 return db.tUser.Find(userId);
 }
 public IEnumerable<tUser> GetAll()
 {
 return db.tUser.ToList();
 }
 public virtual IEnumerable<tUser> GetSome(
 Expression<Func<tUser, bool>> filter = null,
 Func<IQueryable<tUser>, IOrderedQueryable<tUser>> orderBy = null,
 string includeProperties = "")
 {
 IQueryable<tUser> query = db.Set<tUser>();
 if (filter != null)
 {
 query = query.Where(filter);
 }
 foreach (var includeProperty in includeProperties.Split
 (new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries))
 {
 query = query.Include(includeProperty);
 }
 if (orderBy != null)
 {
 return orderBy(query).ToList();
 }
 else
 {
 return query.ToList();
 }
 }
 public void InsertUser(tUser user)
 {
 db.tUser.Add(user);
 }
 public void DeleteUser(object userId)
 {
 var user = db.tUser.Find(userId);
 db.tUser.Remove(user);
 }
 public void UpdateUser(tUser user)
 {
 db.Entry(user).State = EntityState.Modified;
 }
 public void Save()
 {
 db.SaveChanges();
 }
 #region dispose
 private bool disposed = false;
 protected virtual void Dispose(bool disposing)
 {
 if (!this.disposed)
 {
 if (disposing)
 {
 db.Dispose();
 }
 }
 this.disposed = true;
 }
 public void Dispose()
 {
 Dispose(true);
 GC.SuppressFinalize(this);
 }
 #endregion
}

The Unit-Of-Work implementation:

public class UnitOfWork : IDisposable
{
 private DbEntities db = new DbEntities();
 private IUserRepository userRepository;
 public IUserRepository UserRepository
 {
 get
 {
 return userRepository ?? new UsersRepository(db);
 }
 }
 public void Save()
 {
 db.SaveChanges();
 }
 public void Dispose()
 {
 db.Dispose();
 }

The UsersManager window ViewModel:

public class UsersManagerViewModel : ObservableObject
{
 private UnitOfWork unit = new UnitOfWork();
 private string _searchText;
 public string SearchText
 {
 get { return _searchText; }
 set
 {
 if (_searchText != value)
 {
 _searchText = value;
 SelectedUser = null;
 RaisePropertyChanged();
 }
 }
 }
 private UserViewModel _selectedUser;
 public UserViewModel SelectedUser
 {
 get { return _selectedUser; }
 set
 {
 if (_selectedUser != value)
 {
 _selectedUser = value;
 RaisePropertyChanged();
 }
 }
 }
}

UserViewModel:

public class UserViewModel : ObservableObject
{
 private readonly tUser entity;
 public UserViewModel()
 : this(new tUser())
 { 
 }
 public UserViewModel(tUser _entity)
 {
 this.entity = _entity;
 }
 public tUser Model { get { return this.entity; } }
 public string UserName
 {
 get { return entity.Name; }
 set
 {
 entity.Name = value;
 RaisePropertyChanged();
 RaisePropertyChanged("UserFullName");
 }
 }
 public string SurName
 {
 get { return entity.Surname; }
 set
 {
 entity.Surname = value;
 RaisePropertyChanged();
 RaisePropertyChanged("UserFullName");
 }
 }
 public string UserFullName
 {
 get { return entity.Name + " " + entity.Surname; }
 set { RaisePropertyChanged(); }
 }
}

Are my Implementations "correct" as far as respecting the design patterns go? Should I remove the private DbEntities db; field from my repository and wrap each call with a using (var db = new DbEntities()) instead?

• Anthony Fawcett: In practical terms, there's no difference. The connection is pooled by the framework, and it's matched by connection string. So long as that is the same, you'll be re-using the same connection. The framework will also open and close the connection automatically.

Answer 1

UsersRepository

private DbEntities db;
public UsersRepository (DbEntities db)
{
 this.db = db;
} 

You should define DbEntities db as readonly to prevent it beeing changed by accident.

---

The public virtual IEnumerable<tUser> GetSome( Expression<Func<tUser, bool>> filter = null, Func<IQueryable<tUser>, IOrderedQueryable<tUser>> orderBy = null, string includeProperties = "") method should have some parameter validation at least for the includeProperties parameter. This parameter should be checked for null to throw an early exception.

---

This

public void DeleteUser(object userId)
{
 var user = db.tUser.Find(userId);
 db.tUser.Remove(user);
} 

can be simplified by using the GetById() method like so

public void DeleteUser(object userId)
{
 var user = GetById(userId);
 db.tUser.Remove(user);
} 

---

You are using this inconsistently. You are reffering to this only with the disposed variable but never with the db.

---

#region dispose 

Please read Are regions an antipattern or code smell

---

UnitOfWork

private IUserRepository userRepository;
public IUserRepository UserRepository
{
 get
 {
 return userRepository ?? new UsersRepository(db);
 }
} 

In this way userRepository will always be null. Either return each time a new UsersRepository(db); or change the getter to

private IUserRepository userRepository;
public IUserRepository UserRepository
{
 get
 {
 return userRepository ?? (userRepository = new UsersRepository(db));
 }
} 

---

What happens if Dispose() is called twice ? You get a NullReferenceException. So either implement it like you did in the UsersRepository or add a simple null check for db.