Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 6.0 : Breaking Changes #25

Closed
TanvirArjel opened this issue Jun 10, 2022 · 5 comments
Closed

Version 6.0 : Breaking Changes #25

TanvirArjel opened this issue Jun 10, 2022 · 5 comments
Assignees
Milestone

Comments

@TanvirArjel
Copy link
Owner

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 TanvirArjel pinned this issue Jun 10, 2022
@TanvirArjel TanvirArjel self-assigned this Jun 10, 2022
@TanvirArjel TanvirArjel added this to the v6.0.0 milestone Jun 10, 2022
@Krzysztofz01
Copy link

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()?

@TanvirArjel
Copy link
Owner Author

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

@serdarozkan41
Copy link

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 :)

@TanvirArjel
Copy link
Owner Author

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

@HybridSolutions
Copy link

HybridSolutions commented Jun 13, 2022

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
    {
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants