Am i using correctly Unit of Work here? (Entityi Framework 4 POCO)

.net, c#, entity-framework, entity-framework-4, unit-of-work

Solution

This looks basically OK. A few suggestions though:

- You should not let the repository dispose the `TemplateEntities`. The reason for this is that when you need two repositories within one transaction, you have a problem. You should move the responsibility of disposing the `TemplateEntities` to the same level as the `TransactionScope`;

- The `TransactionScope` should be moved to a higher level. Preferably, the `TemplateEntities` should be instantiated within a `TransactionScope`;

- You don't have to create the `Save` wrapper if it does not contain functionality. If you specify the `void SaveChanges()` on the `IUnitOfWork` interface, this will pick up the `SaveChanges` of the `TemplateEntities`;

- Personally I would not have `string GetUserNameByEmail(...)` but rather `User GetUserByEmail(...)` because then this will also serve your purpose and you have the advantage of not having two methods that search by e-mail address when you later need the `User GetUserByEmail(...)`;

- You may want to think about making `ctx` private, or at least a private setter like `public TemplateEntities Ctx { get; private set; }`;

- You could create an abstract repository with methods like the example below. This will save you a lot of dull typing in the long run:

public interface IRepository<TEntity>
{
    void Delete(TEntity entity);

    /* ... */
}

public abstract class AbstractRepository<TEntity> : IRepository<TEntity>
{
    public TemplateEntities ctx;

    public AbstractRepository(IUnitOfWork unit)
    {
        ctx = unit as TemplateEntities;
    }

    protected abstract ObjectSet<TEntity> Entites { get; }

    public virtual void Delete(TEntity entity)
    {
        Entities.Attach(entity);
        ctx.ObjectStateManager.ChangeObjectState(entity, System.Data.EntityState.Deleted);
    }

    /* ... */
}

public interface IUserRepository : IRepository<User>
{
    User GetUser(string username);

    /* ... */
}

public class UserRepository : AbstractRepository<User>, IUserRepository
{
    public UserRepository(IUnitOfWork unit)
        : base(unit)
    {
    }

    protected override ObjectSet<User> Entites
    {
        get { return ctx.Users; }
    }

    public User GetUser(string username)
    {
        return (from u in ctx.Users
                where u.UserName == username
                select u).SingleOrDefault();
    }

    /* ... */
}

Problem

I found some examples of how to create unit of work with ef4, i haven't used di/ioc and i would like to keep things simple and this an example (90% inspired) and i think it's ok but since i am looking at a pattern to use from now on i would like to ask an opinion one last time. ``` public interface IUnitOfWork { void Save(); } public partial class TemplateEntities : ObjectContext, IUnitOfWork { .... public void Save() { SaveChanges(); } } public interface IUserRepository { User GetUser(string username); string GetUserNameByEmail(string email); void AddUser(User userToAdd); void UpdateUser(User userToUpdate); void DeleteUser(User userToDelete); //some other } public class UserRepository : IUserRepository, IDisposable { public TemplateEntities ctx; public UserRepository(IUnitOfWork unit) { ctx = unit as TemplateEntities; } public User GetUser(string username) { return (from u in ctx.Users where u.UserName == username select u).SingleOrDefault(); } public string GetUserNameByEmail(string email) { return (from u in ctx.Users where u.Email == email select u.UserName).SingleOrDefault(); } public void AddUser(User userToAdd) { ctx.Users.AddObject(userToAdd); } public void UpdateUser(User userToUpdate) { ctx.Users.Attach(userToUpdate); ctx.ObjectStateManager.ChangeObjectState(userToUpdate, System.Data.EntityState.Modified); } public void DeleteUser(User userToDelete) { ctx.Users.Attach(userToDelete); ctx.ObjectStateManager.ChangeObjectState(userToDelete, System.Data.EntityState.Deleted); } public void Dispose() { if (ctx != null) ctx.Dispose(); } } ``` And finally ``` public class BogusMembership : MembershipProvider { public MembershipCreateStatus CreateUser(string username, string password, string email, bool autoemail, string fullname) { IUnitOfWork ctx = new TemplateEntities(); using (UserRepository rep = new UserRepository(ctx)) { using (TransactionScope tran = new TransactionScope()) { if (rep.GetUser(username) != null) return MembershipCreateStatus.DuplicateUserName; if (requiresUniqueEmail && !String.IsNullOrEmpty(rep.GetUserNameByEmail(email))) return MembershipCreateStatus.DuplicateEmail; User userToCreate = new User { UserName = username, PassWord = EncodePassword(password), FullName = fullname, Email = email, AutoEmail = autoemail }; try { rep.AddUser(userToCreate); ctx.Save(); tran.Complete(); return MembershipCreateStatus.Success; } catch { return MembershipCreateStatus.UserRejected; } } } } } ``` After getting rid if the IUnitOfWork and IDisposal the CreateUser looks like this: ``` public MembershipCreateStatus CreateUser(string username, string password, string email, bool autoemail, string fullname) { using (TransactionScope tran = new TransactionScope()) { using (TemplateEntities ctx = new TemplateEntities()) { UserRepository rep = new UserRepository(ctx); //OtherRepository rep2 = new OtherRepository(ctx); if (rep.GetUser(username) != null) return MembershipCreateStatus.DuplicateUserName; if (requiresUniqueEmail && !String.IsNullOrEmpty(rep.GetUserNameByEmail(email))) return MembershipCreateStatus.DuplicateEmail; User userToCreate = new User { UserName = username, PassWord = EncodePassword(password), FullName = fullname, Email = email, AutoEmail = autoemail }; try { rep.AddUser(userToCreate); ctx.SaveChanges(); tran.Complete(); return MembershipCreateStatus.Success; } catch { return MembershipCreateStatus.UserRejected; } } } } ```

Original source