EFCore.GenericRepository icon indicating copy to clipboard operation
EFCore.GenericRepository copied to clipboard

Version 6.0 : Breaking Changes

Open TanvirArjel opened this issue 2 years ago • 5 comments

Currently, IRepository command methods are calling await _dbContext.SaveChangesAsync() internally, which arguably breaks the single responsibility pattern. So we have decided to bring the await _dbContext.SaveChangesAsync() out of the following methods:

1. InsertAsync();
2. UpdateAsync();
3. DeleteAsync();

The above methods will be replaced with the following methods:

1. Add();
2. Update();
3. Remove();

And to persist the changes to the database, the user has to call the await _repository.SaveChangesAsync() explicitly.

Currently:

_repository.InsertAsync(employee);

In from version 6.0:

_repository.Add(employee);
await _repository.SaveChangesAsync();

However, what should be the next method names:

1. Add();
2. Update();
3. Remove();

or

1. Insert();
2. Update();
3. Delete();

Please share your thoughts.

TanvirArjel avatar Jun 10 '22 16:06 TanvirArjel

The Add(), Update(), Remove() approach seems fine. But i think that the SaveChanges() method should have a different name. EfCore SaveChanges() method works globally and is finalizing the transaction for all tracked entities. To make the ,,save'' method look more repository (which is representing a single entity) related, we can choose a different name, mayby Commit()?

Krzysztofz01 avatar Jun 10 '22 16:06 Krzysztofz01

Thank you for your opinion:

EfCore SaveChanges() method works globally and is finalizing the transaction for all tracked entities. To make the ,,save'' method look more repository (which is representing a single entity) related, we can choose a different name, mayby Commit()?

However, Our IRepository methods are generic, so you can add multiple command operations for multiple entities one by one and SaveCahnges all at once. That's why we are naming it SaveChangesAsync().

TanvirArjel avatar Jun 10 '22 16:06 TanvirArjel

While sole responsibility is important, it is not a requirement.

Names

  1. Add();
  2. Update();
  3. Delete(); instead of changing
  4. InsertAsync(); with save
  5. Add(); without saving
  6. UpdateAsync(); with save
  7. Update(); without saving
  8. DeleteAsync(); with save
  9. Delete(); without saving

Wouldn't it be much more reasonable to continue as ?

Or could this be an option?

services.AddGenericRepository<YourDbContext>().AutoSave(true);

I understand that principles are important. but this is not a client project, is it really that important? Just asking :)

serdarozkan41 avatar Jun 10 '22 17:06 serdarozkan41

@serdarozkan41 Thank you so much for your opinion. If we keep all the methods, it will create a lot of confusion for the end users. I would like to keep things simple.

TanvirArjel avatar Jun 11 '22 02:06 TanvirArjel

I don't see any disadvantage on the new approach, as long as multiple operations can still work in case of using transactions. Don't like the .Commit() since EF Core uses this to commit transactions and it would be confusing. I think the closest it ressembles to EF Core the better, so Add(), Remove() and Update() are my choices.

By the way, you have this typo "TDbConext" in some files that should be corrected until further versions are out.

/// <typeparam name="TDbConext">The type of the <see cref="DbContext"/>.</typeparam>
    public interface IQueryRepository<TDbConext> : IQueryRepository
    {
    }

HybridSolutions avatar Jun 13 '22 14:06 HybridSolutions