My branch #322
My branch #322
Conversation
|
|
||
| @RestController | ||
| @RequestMapping("/categories") | ||
| public class CategoryController { |
There was a problem hiding this comment.
it's not a mistake, but you can use @requiredargsconstructor instead of creating a constructor
| @ManyToOne(fetch = FetchType.LAZY) | ||
| private Category category; | ||
|
|
||
| @Override |
There was a problem hiding this comment.
Yes, but I relied on the materials described in our lecture:
| public class CategoryServiceImpl implements CategoryService { | ||
| private final CategoryRepository categoryRepository; | ||
|
|
||
| public CategoryServiceImpl(CategoryRepository categoryRepository) { |
|
|
||
| @Override | ||
| public Category update(Category category, Long id) { | ||
| Category oldCategory = categoryRepository.findById(id) |
There was a problem hiding this comment.
I prefer to use getById method, Category oldCategory= this.getById(id);
There was a problem hiding this comment.
I don't know what would be better here, I thought that when updating an entity, I should check whether it is in the database with this ID and then update it.
There was a problem hiding this comment.
I think @ykhmaruk meant that you have code duplication here and exactly the same 'find' operation was implemented within your service's getById() method, so you can reuse it here
There was a problem hiding this comment.
Yes, thank you, I later figured out what he meant and really realized that it would be better, or at least avoid duplication of code
| .orElseThrow(() -> new NoSuchElementException("Can't find the category by " | ||
| + "id: " + id)); | ||
| oldCategory.setName(category.getName()); | ||
| oldCategory.setId(id); |
There was a problem hiding this comment.
The ID you are trying to set is already set, it is a redundant line
|
|
||
| @Override | ||
| public Product update(Product product, Long id) { | ||
| Product oldProduct = productRepository.findById(id) |
There was a problem hiding this comment.
I understand that this is also a working approach, but my version is more readable for me, but if the mentor tells me to correct it, I will do it)
|
|
||
| @Override | ||
| public Category update(Category category, Long id) { | ||
| Category oldCategory = categoryRepository.findById(id) |
There was a problem hiding this comment.
I think @ykhmaruk meant that you have code duplication here and exactly the same 'find' operation was implemented within your service's getById() method, so you can reuse it here
No description provided.