-
Notifications
You must be signed in to change notification settings - Fork 343
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
Implemented all needed repositories/services/controllers #323
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very good job!)
@Table(name = "products") | ||
public class Product { | ||
@Id | ||
@GeneratedValue(strategy = GenerationType.IDENTITY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's improve your solution :)
} | ||
|
||
@GetMapping("/{id}") | ||
public CategoryResponseDto getByID(@PathVariable Long id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public CategoryResponseDto getByID(@PathVariable Long id) { | |
public CategoryResponseDto get(@PathVariable Long id) { |
Category category = service.add(mapper.mapToModel(dto)); | ||
return mapper.mapToDto(category); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's be consistent in your code style. Either use additional variables everywhere or avoid creating it
} | ||
|
||
@GetMapping("/{id}") | ||
public ProductResponseDto getByID(@PathVariable Long id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public ProductResponseDto getByID(@PathVariable Long id) { | |
public ProductResponseDto get(@PathVariable Long id) { |
} | ||
|
||
@GetMapping("/categories") | ||
public List<ProductResponseDto> getByCategorie(@RequestParam List<String> categoryNames) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public List<ProductResponseDto> getByCategorie(@RequestParam List<String> categoryNames) { | |
public List<ProductResponseDto> getByCategory(@RequestParam List<String> categoryNames) { |
import java.math.BigDecimal; | ||
import lombok.Data; | ||
|
||
@Data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not use @Data
annotation. It's better to use Getter Setter etc. Please fix the same issue in all places
+ "id=" + id | ||
+ ", title='" + title + '\'' | ||
+ ", price=" + price | ||
+ ", category=" + category |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please pay attention to @V-Solianyk comment and remove 'category' from toString method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
No description provided.