3
\$\begingroup\$

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; }
}
\$\endgroup\$
  • 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 Mar 7 '17 at 12:20
  • \$\begingroup\$ Any progress on this issue? It's always nice to get some feedback :) \$\endgroup\$ – Gert Arnold Sep 5 '17 at 14:25
1
\$\begingroup\$

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.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

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();
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.