The company I work for does something like this:
static Employee getEmployee(string ssn){
MyEntity context = null;
Employee employee;
try{
context = new MyEntity();
employee = context.Employee.Where(X => X.ssn.Equals(ssn));
}
catch (Exception exc){
LogLibrary.WriteLog(exc.Message);
}
finally{
if (context != null){
context.Database.Connection.Close();
}
}
return employee;
}
static Employee addEmployee(Employee emp){
MyEntity context = null;
Employee employee;
try{
context = new MyEntity();
context.Employee.Add(e);
}
catch (Exception exc){
LogLibrary.WriteLog(exc.Message);
}
finally{
if (context != null){
context.Database.Connection.Close();
}
}
}
And this is the code I want to implement:
Employee myNewEmployee = DBClass.getEmployee("12345");
myNewEmployee.name = "John";
DBClass.AddEmployee(myNewEmployee);
But I obviously receive the following exception: An entity object cannot be referenced by multiple instances of IEntityChangeTracker
So the company recommended me to do the following:
Employee myNewEmployee = DBClass.getEmployee("12345");
Employee myNewEmployee2 = new Employee();
// copy all the fields from myNewEmployee to myNewEmployee2
myNewEmployee2.name = "John";
DBClass.AddEmployee(myNewEmployee2);
But I find this so unefficient. Should we use a single static context for the whole application? (It's an ASPx project with master page). Where can I read more about "how to use context"? Thank you so much.
CodePudding user response:
Since the code is working with completely separate DbContext instances for each call, it is technically possible to update the instance to copy and save it as a new employee after ensuring all unique values and keys are updated. However, your existing code is leaving the entity tracking references orphaned by disposing the DbContext. Code like that getEmployees should either be loading entities in a detached state with AsNoTracking()
or detaching the instance from the DbContext before the context is disposed. The method can also be simplified to remove the finally
block to handle disposal by using a using
block:
static Employee getEmployee(string ssn)
{
using(var context = new MyEntity())
{
try
{
return context.Employee.AsNoTracking().Single(X => X.ssn.Equals(ssn));
}
catch (Exception exc)
{
LogLibrary.WriteLog(exc.Message);
}
}
return null;
}
Alternatively you can detach the entity before returning it:
try
{
var employee = context.Employee.Single(X => X.ssn.Equals(ssn));
context.Entry(employee).State = EntityState.Detached;
return employee;
}
using
blocks take care of disposing instances within a scope. One missed finally
block and you have an undisposed DbContext instance.
When that initial entity is not tracked by the now disposed DbContext, you can have code like:
var employee = getEmployee("12345");
employee.SSN = "54321";
employee.Name = "John";
using(var context = new MyEntity())
{
context.Employees.Add(employee);
context.SaveChanges();
}
Another alternative is to clone the entity.There are a few ways to do this including manually copying the values across or leveraging Automapper to copy the values across. This can be configured to ignore copying keys and unique values. At the very basic:
var config = new MapperConfiguration(cfg => cfg.CreateMap<Employee, Employee>());
var mapper = config.CreateMapper();
var sourceEmployee = getEmployee("12345");
var newEmployee = mapper.Map<Employee>(sourceEmployee);
newEmployee.SSN = "54321";
newEmployee.Name = "John";
using(var context = new MyEntity())
{
context.Employees.Add(newEmployee);
context.SaveChanges();
}
This code would need to ensure that the primary key value and any unique constraints are updated. If the employee table has an EmployeeId PK and that key is set up as an Identity then that should be covered automatically. Otherwise if the PK is something like the SSN you will need to ensure that is a new and unique value before saving. To do that you should first check the database to ensure the new SSN is unique:
using(var context = new MyEntity())
{
if (!context.Employees.Any(x => x.SSN == newEmployee.SSN))
{
context.Employees.Add(newEmployee);
context.SaveChanges();
}
else
{
// handle that the new SSN is already in the database.
}
}
Regarding just using a single static DbContext: No, that is not a good idea with EF. DbContexts by default track every instance they load unless explicitly told not to. This means the longer they are alive, the more instances they track, consuming memory and causing performance drops as EF will continually check across it's known tracked instances to see if it should return that rather than a new instance pulled from the database. It still runs the queries in most cases, so dealing with tracked instances does not save performance like you might think comparing the behaviour to caching. Normally you would want multiple calls though to be associated with a single DbContext instance so having the DbContext too finely scoped makes it less flexible. For example if you wanted to update a Position in a company and associate it with an employee, having a getEmployee() method that scoped it's own DBContext can actually have unintended consequences such as this example:
using (var context = new MyEntity())
{
var position = context.Positions.Single(x => x.PositionId == positionId);
var employee = getEmployee(ssn);
position.Employee = employee;
context.SaveChanges();
}
What this can end up resulting with is an error about a duplicate constraint on attempting to insert a new Employee, or it will insert a completely new clone of the Employee record. (If the Employee is configured with an Identity for it's PK) The reason for this is that the Position is being managed by one instance of the DbContext while the getEmployee() was using a completely separate DbContext instance. The position doesn't know that "employee" is an existing record and treats it like a brand new one. The proper way to ensure these instances are associated together is to ensure they are both associated with the same DbContext instance:
using (var context = new MyEntity())
{
var position = context.Positions.Single(x => x.PositionId == positionId);
var employee = context.Employees.Single(x => x.SSN == ssn);
position.Employee = employee;
context.SaveChanges();
}
Or else ensuring that both this code and getEmployee are injected with the same DbContext
instance rather than scoping it within the methods. (I.e. dependency injection) Working with detached instances like your code is structured is possible but it can get quite messy so be cautious.