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 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
.
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
.
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
.
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
.