Feat/create event with categories and ticket typed #74
Conversation
| public async Task<bool> CheckIfCategoriesExistAsync(List<Category> categories) | ||
| { | ||
| var dbCategories = await _tickApiDbContext.Categories.ToListAsync(); | ||
| return categories.All(c => dbCategories.Any(cdb => cdb.Name == c.Name)); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can we apply filter here before calling ToListAsync()? I think what happens now is all categories are fetched into memory and then they are filtered. If we do something like _tickApiDbContext.Categories.Where(c => categories.Include(c.Name)).Count() == categories.Count (the actual implementation may differ, but you get the idea - filter before calling ToListAsync() or any other function that actually fetches the records into memory) we don't fetch all categories but rather filter them on the sql level. I know in our case it's not gonna change much as we don't have many categories but I think it's something we should do anyway.
I also think in functions like this we should make arguments as generic as possible for future use, so in this case there is nothing stopping us from using IEnumerable<Category> instead of List<Category>.
Another thing is that I believe this kind of functionality should be placed in CategoryService rather than CategoryRepository. You can use GetCategories function from CategoryRepository and use the result of it in CategoryService for further data processing, I just feel like this is too high-level logic to put it in repository, which should be just responsible for data transfer between our app and db.
There was a problem hiding this comment.
I agree with the points you made in the review but I believe the last point is contradictory to the first one. In the first one you say that we should change the data fetching so we fetch to the memory already filtered data and in the third point you say that I should use GetCategories which is a function that fetches all the data without applying filters. I mean the third point has the problem that you described in the first point. Which logic should I use then? I am more in favour of the one with changing it into service because, as you observed, there are not many categories and organizers are not allowed to add custom ones so the category table won't grow much. I would like to here your ideas @kubapoke @kTrzcinskii
There was a problem hiding this comment.
The IQueryable<T> represents the SQL query, it actually doesn't fetch data until you call ToListAsync on it or some other similar method. So if you call the GetCategories you can apply filters on them without having to fetch all the data
…//github.com/Resellio/api into feat/CreateEventWithCategoriesAndTicketTyped
No description provided.