I have a table named Asset
and each Asset
has schedules. I create a method like this for copying an entity:
public bool Copy(CopyAssetViewModel copyAssetViewModel,out string message)
{
var source = _equipments.AsNoTracking().FirstOrDefault(row => row.Id == copyAssetViewModel.AssetId);
if (source == null) return false;
int count = source.Schedules.Count;
Asset newAsset = new Asset();
Guid tempId = newAsset.Id;
newAsset = source;
newAsset.Id = tempId;
newAsset.AssetCode = copyAssetViewModel.AssetCode;
if (copyAssetViewModel.CopySchedules)
{
newAsset.Schedules = source.Schedules.ToList();
newAsset.Schedules.ToList().ForEach(row =>
{
Schedule newSchedule = new Schedule();
Guid scheduleId = newSchedule.Id;
newSchedule = row;
newSchedule.Id = scheduleId;
newSchedule.AssetId = tempId;
});
}
else
newAsset.Schedules.Clear();
_uow.MarkAsAdded(newAsset, _userManager.GetCurrentUserId());
_uow.SaveChanges();
return true;
}
All entities inherit BaseEntity
and I have a constructor for creating a new Id
like this:
public BaseEntity()
{
Id = SequentialGuidGenerator.NewSequentialGuid();
CreatedOn = DateTime.Now;
}
But when I use this code:
newAsset.Schedules = source.Schedules.ToList();
Id
and createdOn
are the same. I've gotten an error for a duplicate key. For solve this I used these lines:
Schedule newSchedule = new Schedule();
Guid scheduleId = newSchedule.Id;
newSchedule = row;
newSchedule.Id = scheduleId;
And I have another field that needs to update like: CreatedByUserId
.
I think my code is very poor. Can everyone give me an idea to improve it?
copyAssetViewModel
:
public class CopyAssetViewModel
{
public Guid AssetId { get; set; }
public bool CopySchedules { get; set; }
public int AssetCode { get; set; }
}
-
1\$\begingroup\$ Welcome to Code Review! Could you please reorder the contents of the post so that the working code stays together, and explanations come after or before it. Usually what people do is copy to their IDE and deal with it there. Splitting working code around makes it less comfortable to do. \$\endgroup\$Incomputable– Incomputable2017年03月07日 12:20:53 +00:00Commented Mar 7, 2017 at 12:20
-
\$\begingroup\$ Any progress on this issue? It's always nice to get some feedback :) \$\endgroup\$Gert Arnold– Gert Arnold2017年09月05日 14:25:36 +00:00Commented Sep 5, 2017 at 14:25
2 Answers 2
It appears that _equipments.AsNoTracking().FirstOrDefault
gets some objects that have a unique ID
When you _uow.MarkAsAdded(newAsset, _userManager.GetCurrentUserId());
that collection requires unique ID. You go through round about way to to create a new ID so that you don't get a conflict.
Same thing for both Asset ID and Schedule ID.
All this is doing is assign a new unique ID.
Schedule newSchedule = new Schedule();
Guid scheduleId = newSchedule.Id;
newSchedule = row;
newSchedule.Id = scheduleId;
newSchedule is a reference to row so newSchedule.Id = scheduleId;
is just assigning a new ID to row
It someone is reading that different please comment.
It appears you changing code to get rid of error messages without fully understanding the code and objectives.
First, you can copy an object easily by getting it with AsNoTracking
and then Add()
-ing it to the context. Even child objects will be added:
So, leaving the Id values for a moment, the core of the copying code could be nothing but:
var source = _equipments.AsNoTracking()
.Include(e => e.Schedules)
.FirstOrDefault(row => row.Id == copyAssetViewModel.AssetId);
_uow.MarkAsAdded(source);
... assuming that _uow.MarkAsAdded
simply Add()
s the entity to the context. Now EF will see source
and source.Schedules
as new objects and (try to) insert them on SaveChanges
.
So how to assign the Id
and CreatedOn
values?
You could do that in a very generic way that will solve this for any BaseEntity
you add, not only copied ones, even removing the need of this BaseEntity
constructor. It's by overriding SaveChanges
in your DbContext
subclass:
public override int SaveChanges()
{
foreach (var entry in ChangeTracker.Entries<BaseEntity>()
.Where(e => e.State == EntityState.Added).ToList())
{
entry.Entity.Id = SequentialGuidGenerator.NewSequentialGuid();
entry.Entity.CreatedOn = DateTime.Now;
}
return base.SaveChanges();
}