solution to jv-salary-info #1354
Conversation
There was a problem hiding this comment.
It looks like there are a couple of critical issues that need to be addressed: the use of a LinkedHashMap and the need to make the formatter a constant. Once these are resolved, your code will be in great shape! Keep up the good work and feel free to reach out if you need any help. 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -1,7 +1,47 @@ | |||
| package core.basesyntax; | |||
|
|
|||
| import java.time.LocalDate; | |||
| import java.time.format.DateTimeFormatter; | |||
| import java.util.LinkedHashMap; | |||
There was a problem hiding this comment.
Using a LinkedHashMap is not recommended according to the checklist. Try to solve the task without using Map or any other similar data structures.
| public class SalaryInfo { | ||
| private DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd.MM.yyyy"); |
There was a problem hiding this comment.
The formatter should be a constant field as per the checklist. Consider making it a constant.
There was a problem hiding this comment.
Great job on following the checklist by using System.lineSeparator() and defining the formatter as a constant! 🎉 However, remember to use descriptive variable names for better code readability, like renaming 'n' to 'employeeName'. Keep up the good work and learn from these suggestions to enhance your coding skills!
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| public class SalaryInfo { | ||
| private static DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("dd.MM.yyyy"); |
There was a problem hiding this comment.
The formatter is correctly defined as a constant field, which is a good practice according to the checklist.
| int money = 0; | ||
| for (String record : data) { | ||
| String[] arr = record.split(" "); | ||
| String n = arr[1]; |
There was a problem hiding this comment.
The variable name 'n' is not informative. According to the checklist, variable names should be descriptive. Consider renaming it to something like 'employeeName'.
| StringBuilder builder = new StringBuilder(); | ||
| builder.append("Report for period ").append(from).append(" - ").append(to); | ||
| for (String record : results) { | ||
| builder.append(System.lineSeparator()) |
There was a problem hiding this comment.
Good use of System.lineSeparator() for handling line separators, as recommended in the checklist.
No description provided.