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?

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.