added solution of task #1612
added solution of task #1612
Conversation
There was a problem hiding this comment.
Thank you for your hard work on the WorkWithFile class! 😊 Your implementation is close to meeting the task requirements, but there are a few critical issues that need to be addressed before we can approve the pull request:
-
Exception Handling: The code currently does not handle potential exceptions when parsing integers from the CSV file. This could lead to runtime errors if the file contains invalid data. Please add error handling for
Integer.parseInt(lineSplit[1].trim())to manage potentialNumberFormatException. -
Method Length and Responsibilities: The
getStatisticmethod is handling multiple responsibilities, making it longer than recommended. Consider breaking it down into smaller methods, each with a single responsibility, to improve readability and maintainability. -
Use of Constants: Strings like "supply" and "buy" are used directly in the code. Declaring these as constants will enhance code readability and maintainability.
-
Method Naming: The current method name
getStatisticmight not fully convey its operations. Consider renaming it to better reflect its functionality. -
Access Modifiers: If
getStatisticis only used within the same class, consider using a more restrictive access modifier to adhere to encapsulation principles.
Once these issues are addressed, your code will be in great shape! Keep up the great work, and feel free to reach out if you have any questions or need further assistance. 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
| String line; | ||
| while ((line = bufferedReader.readLine()) != null) { | ||
| String[] lineSplit = line.split(","); | ||
| int value = Integer.parseInt(lineSplit[1].trim()); |
There was a problem hiding this comment.
Consider adding error handling for Integer.parseInt(lineSplit[1].trim()) to manage potential NumberFormatException if the CSV file contains non-numeric values.
There was a problem hiding this comment.
Review Summary
Great job on improving the WorkWithFile class! 🎉 You've successfully implemented exception handling for NumberFormatException and used try-with-resources for file operations, which are excellent practices. However, there are a few areas that could use some refinement:
-
Method Length and Responsibilities: The
getStatisticmethod is currently handling multiple responsibilities, such as reading from a file, processing data, and writing to a file. Breaking it down into smaller methods, each with a single responsibility, would enhance readability and maintainability. -
Use of Constants: The strings "supply" and "buy" are used directly in the code. Declaring these as constants would improve code readability and maintainability.
-
Access Modifiers: If the
getStatisticmethod is only used within the same class, consider using a more restrictive access modifier to adhere to encapsulation principles.
These improvements will not only make your code cleaner but also more robust and easier to maintain. Keep up the good work, and continue refining your skills! Remember, addressing these points will help you avoid similar issues in the future and enhance your learning experience. 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| public void getStatistic(String fromFileName, String toFileName) { | ||
| // Variables to store the total supply and buy amounts | ||
| int supply = 0; | ||
| int buy = 0; | ||
| int result; | ||
| // Step 1: Read the CSV file | ||
| try (BufferedReader bufferedReader = new BufferedReader(new FileReader(fromFileName))) { | ||
| String line; | ||
| while ((line = bufferedReader.readLine()) != null) { | ||
| String[] lineSplit = line.split(","); | ||
| try { | ||
| int value = Integer.parseInt(lineSplit[1].trim()); | ||
| String lineEach = lineSplit[0].trim(); | ||
| if (lineEach.equals("supply")) { | ||
| supply += value; | ||
| } else if (lineEach.equals("buy")) { | ||
| buy += value; | ||
| } | ||
| } catch (NumberFormatException e) { | ||
| throw new RuntimeException("Invalid number format", e); | ||
| } | ||
| } | ||
| result = supply - buy; | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("Error in reading CSV file", e); | ||
| } | ||
|
|
||
| StringBuilder builderReport = new StringBuilder(); | ||
| builderReport.append("supply,").append(supply).append(System.lineSeparator()); | ||
| builderReport.append("buy,").append(buy).append(System.lineSeparator()); | ||
| builderReport.append("result,").append(result).append(System.lineSeparator()); | ||
|
|
||
| try (BufferedWriter bufferedWriter = new BufferedWriter(new FileWriter(toFileName))) { | ||
| bufferedWriter.write(builderReport.toString()); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("Error in writing file", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
The getStatistic method is still handling multiple responsibilities, such as reading from a file, processing data, and writing to a file. Consider breaking it down into smaller methods, each with a single responsibility, to improve readability and maintainability.
| if (lineEach.equals("supply")) { | ||
| supply += value; | ||
| } else if (lineEach.equals("buy")) { |
There was a problem hiding this comment.
The strings "supply" and "buy" are used directly in the code. Consider declaring these as constants to enhance code readability and maintainability.
No description provided.