Conversation
| bool userCanNotCreateCatalog | ||
| ) | ||
| { | ||
| if (userCanNotCreateCatalog) |
There was a problem hiding this comment.
Tutaj to albo nie jest potrzebne albo się źle się nazywa - userCanNotCreateCatalog - to w sumie nic nie mówi jakie są zasady
| public string Name { get; private set; } | ||
| public string Genre { get; private set; } | ||
| public string Description { get; private set; } | ||
| public List<Book> Books { get; set; } |
There was a problem hiding this comment.
Moze nie ma potrzeby trzymania tutaj listy Books na wricie
| { | ||
| if (updater == Guid.Empty) | ||
| throw new UserCanNotUpdateCatalogException(); | ||
| if (userCanNotUpdateCatalog) |
There was a problem hiding this comment.
userCanNotUpdateCatalog - ten parametr nic nie mówi co to sprawdza - trzeba lepiej nazwać
| { | ||
| Task AddAsync(Catalog catalog); | ||
|
|
||
| Task<bool> UserCanNotAddCatalogAsync(Guid bookStoreId); |
There was a problem hiding this comment.
Te nazwy tutaj też niewiele mówią - nie wiadomo co to sprawdza póki się nie wejdzie do implementacji na bazie
| if (book is null) | ||
| throw new BookNotFoundException(); | ||
|
|
||
| await catalogRepository.AddBookToCatalogAsync(book, catalog); |
There was a problem hiding this comment.
albo catalog.AddBook(book) albo book.SetCatalog(catalog)
There was a problem hiding this comment.
Nie powinniśmy przekładać odpowiedzialności za procesy biznesowe na repozytorium
There was a problem hiding this comment.
Do przemyślenia czy czasem nie zwrobić tak że Book ma public Catalog, a katalog nie ma List - bo za każdym update byśmy musieli to zaciągać z bazy
| if (catalog.CreatedBy != context.Identity.Id) | ||
| throw new UserCanNotRemoveBookFromCatalogException(); | ||
|
|
||
| await catalogRepository.RemoveBookFromCatalogAsync(book, catalog); |
There was a problem hiding this comment.
Tu też odpowiedzialność za proces jest na repo
| builder | ||
| .HasMany<Book>() | ||
| .WithMany() | ||
| .UsingEntity(j => j.ToTable("CatalogBooks")); |
There was a problem hiding this comment.
Raczej nie ma potrzeby tego tak robić, automatycznie będziemy mieć
There was a problem hiding this comment.
Ale to nie jest błąd
| } | ||
| public async Task<bool> UserCanNotAddCatalogAsync(Guid bookStoreId) | ||
| { | ||
| return await dbContext.Catalogs.CountAsync(x => x.BookStoreId == bookStoreId) >= 5; |
There was a problem hiding this comment.
Logika na repo - to powinno byc zwracane ale nie sprawdzane tutaj
| dbContext.Catalogs.Update(catalog); | ||
| await dbContext.SaveChangesAsync(); | ||
| } | ||
| public Task<bool> UserCanNotUpdateCatalogAsync(DateTime lastUpdated, DateTime now) |
There was a problem hiding this comment.
Ta metoda wogóle nie powinna być w repozytorium - sprawdzanie warunków biznesowych zostawmy w domenie
| } | ||
|
|
||
|
|
||
| public async Task AddBookToCatalogAsync(Book book, Catalog catalog) |
There was a problem hiding this comment.
To potem też pewnie będzie do zmiany
2 participants
No description provided.