feat(task 2): add Statistics feature#36
Conversation
MashroomMole
requested review from
IgorGursky,
Melancholic,
algorithm108 and
trioletas
Test Results23 tests 23 ✅ 2s ⏱️ Results for commit 45532c8. ♻️ This comment has been updated with latest results. |
MashroomMole
changed the title
MashroomMole
changed the title
|
|
||
| if (ex instanceof MethodArgumentNotValidException invalidArgumentException) { | ||
| errors = | ||
| invalidArgumentException.getBindingResult().getAllErrors().stream() |
There was a problem hiding this comment.
It looks like error extraction is the same, why then this IF is required?
|
|
||
| @RestControllerAdvice | ||
| public class ExceptionHandler { | ||
| @org.springframework.web.bind.annotation.ExceptionHandler({ |
There was a problem hiding this comment.
I believe this can be import shortened
| createdBy, | ||
| now, | ||
| now, | ||
| completed ? Instant.now() : null); |
|
|
||
| @JsonCreator | ||
| public static TodoStatisticsFormatEnum fromString(String value) { | ||
| for (TodoStatisticsFormatEnum format : TodoStatisticsFormatEnum.values()) { |
There was a problem hiding this comment.
Suggest to convert to Java Stream API
| } | ||
|
|
||
| @JsonValue | ||
| public String getValue() { |
| dto.setTotalTodos(total); | ||
| dto.setCompletedTodos(completed); | ||
| dto.setPendingTodos(pending); | ||
| dto.setUserStats(userStats); |
There was a problem hiding this comment.
Why don't you consider creating a separate class for user stats data?
| request.setFormat(format); | ||
| } | ||
|
|
||
| private Map<String, List<Todos>> initiateTodosMap() { |
There was a problem hiding this comment.
Read out on web, there is a way in modern Java to handle this scenario without prior initialization.
There's a method in Map called putIfAbsent, where you pass default value if Key does not exist in map. Maybe that's suitable in this case.
| @TodoStatisticsValidDateRange | ||
| public class TodoStatisticsRequest { | ||
| @Parameter(example = "2025-09-01T10:30:00Z") | ||
| private Instant from; |
There was a problem hiding this comment.
Consider switching to LocalDateTime, in case you don't care about server time or timezone issue in this example
2 participants
Summary:
Detailed:
