Unit of work Repository pattern DBcontext EF-Entities and ninject

I am totally confused while implementing Repository pattern with unit of work and Entity framework because I have seen tons of examples. Please suggest whether I am on the right track or not.

public interface IUnitOfWork: IDisposable
{
    void Commit();
    void Dispose(bool disposing);
}

Here, Medevenet is EF DBContext. For each repo I have property in Unit or Work classes.

```
public class UnitOfWork : IUnitOfWork
{
private MedevnetDBEntities context;

private PostRepository postRepository;

public UnitOfWork(string connectionString)
{
context = new MedevnetDBEntities(connectionString);
}

public PostRepository PostRepository
{
get
{
if (this.postRepository == null)
{
this.postRepository = new PostRepository(context);
}
return postRepository;
}
}

public void Commit()
{
context.SaveChanges();
}

private bool disposed = false;
private string p;

public virtual void Dispose(bool disposing)
{
if (!this.disposed)
{
if (disposing)
{
context.Dispose();
}
}
this.disposed = true;
}

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
}

public interface IRepository where T : class
{
void Insert(T entity);
void Delete(object id);
void Delete(T entityToDelete);
void Update(T entityToUpdate);
}

public class Repository : IRepository where T : class
{
protected MedevnetDBEntities context;
internal DbSet dbSet;

public Repository(MedevnetDBEntities context)
{
this.context = context;
this.dbSet = context.Set();
}
public virtual IEnumerable GetWithRawSql(string query, params object[] parameters)
{
return dbSet.SqlQuery(query, parameters).ToList();
}

public virtual IEnumerable Get(
Exp

-
IUnitOfWork

-
as IDisposable is only relevant for an instance of a class, it shouldn't be implemented by the IUnitOfWork interface. This would only add an unnecessary constraint to the interface.

-
Dispose(bool) shouldn't be there. This is a method which should only exist in the class and also should be protected. By using it in an interface it automatically will be public.

-
UnitOfWork

• dead code should be removed (here private string p;)

• if you use this you should be consistent (see coonstructor and PostRepository property)

-
IRepository

• the names of the input parameters aren't consistent. I suggest changing entityToDelete and entityToUpdate to entity as the action which should happen is expressed in the method name and therefor shouldn't be reflected in the parameter name.

-
Repository.Get()

-
you could make your code more readable, by extracting the Split() out of the loop definition.

-
you can remove the last else, as this is not needed at all.

public virtual IEnumerable Get(
    Expression> filter = null,
    Func, IOrderedQueryable> orderBy = null,
    string includeProperties = "")
{
    IQueryable query = dbSet;

    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();
    }
}

will become

public virtual IEnumerable Get(
    Expression> filter = null,
    Func, IOrderedQueryable> orderBy = null,
    string includeProperties = "")
{
    IQueryable query = dbSet;

    if (filter != null)
    {
        query = query.Where(filter);
    }

    var properties = includeProperties.Split(new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries);

    foreach (var includeProperty in properties)
    {
        query = query.Include(includeProperty);
    }

    if (orderBy != null)
    {
        return orderBy(query).ToList();
    }

    return query.ToList();
}

StackExchange Code Review Q#71437, answer score: 2