SOLID #1256

alisahoys · 2024-12-05T11:30:28Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this project! 😊 While your code is well-structured and mostly meets the requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

Absolute Path Usage: The use of an absolute path in Main.java for reading the CSV file is not recommended. Please switch to a relative path to ensure the project can be run by anyone without modifications.
Package Structure: The current package structure does not adhere to the recommended organization. Consider organizing your code into packages like db, model, service, service.impl, and strategy for better maintainability and clarity.
Handling Purchase Operation: The PurchaseOperation class currently does not check if the resulting balance is positive before subtracting the quantity. This is crucial to prevent invalid storage states. Please ensure that a RuntimeException is thrown if the balance becomes negative.
Input and Output File Location: Ensure that input and output files are placed in the src/main/resources folder as per the checklist.

By addressing these issues, your code will be more robust and aligned with best practices. Keep up the great work, and I'm confident you'll nail it with these adjustments! 🚀

✨ 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.

mateacademy-ai-mentor · 2024-12-05T11:33:36Z

src/main/java/core/basesyntax/PurchaseOperation.java

+
+public class PurchaseOperation implements OperationHandler {
+    public void handle(FruitTransaction transaction) {
+        Storage.subtract(transaction.getFruit(), transaction.getQuantity());


Ensure that the resulting balance in the storage is not negative after a purchase. If it is, throw a RuntimeException as per the checklist requirement: 'Handling Purchase operation. Check result balance before saving it in the Storage - it should be positive. Throw RuntimeException in case the balance is negative.'

mateacademy-ai-mentor · 2024-12-05T11:33:37Z

src/main/java/core/basesyntax/Storage.java

+    public static void subtract(String fruit, int quantity) {
+        fruits.put(fruit, fruits.getOrDefault(fruit, 0) - quantity);


Ensure that the resulting quantity after subtraction is not negative. If it is, throw a RuntimeException to prevent invalid storage states. This aligns with the checklist requirement: 'Handling Purchase operation. Check result balance before saving it in the Storage - it should be positive. Throw RuntimeException in case the balance is negative.'

mateacademy-ai-mentor

Great job on your progress! 🎉 While there are still a few critical issues to address, such as ensuring file paths are relative for portability and adding necessary checks to prevent invalid states or exceptions, your implementation is on the right track.

Here are some key points to focus on:

File Paths: Ensure both input and output file paths are relative to make your project more portable. Consider using paths like "src/main/resources/input.csv" and "src/main/resources/finalReport.csv".
Data Validation: Add validation in DataConverterImpl.java to handle potential parsing errors, ensuring that the input data is well-formed and any exceptions are managed gracefully.
Operation Handling: In OperationStrategyImpl.java, add checks to handle cases where an operation might not have a corresponding handler to prevent NullPointerException.
Storage Integrity: Update the subtract method in Storage.java to check for negative quantities and throw an exception if necessary, maintaining data integrity.

These improvements will enhance the robustness and reliability of your code. Keep up the good work, and make sure to address these issues in future iterations to avoid similar problems. Your dedication to learning and improving is commendable! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-12-12T16:07:01Z

src/main/java/core/basesyntax/Main.java

+        String finalReport = reportGenerator.getReport();
+
+        MyFileWriter fileWriter = new FileWriterImpl();
+        fileWriter.write(finalReport, "src/main/resources/finalReport.csv");


The output file path should be relative to ensure the project can be run by anyone without modifications. Consider using a path like "src/main/resources/finalReport.csv".

mateacademy-ai-mentor · 2024-12-12T16:07:01Z

src/main/java/core/basesyntax/db/Storage.java

+    }
+
+    public static void subtract(String fruit, int quantity) {
+        fruits.put(fruit, fruits.getOrDefault(fruit, 0) - quantity);


The subtract method should check if the resulting quantity is negative before updating the storage. Consider throwing a RuntimeException if the subtraction would result in a negative quantity, to prevent invalid storage states.

mateacademy-ai-mentor · 2024-12-12T16:07:01Z

src/main/java/core/basesyntax/service/impl/DataConverterImpl.java

+        for (int i = 1; i < rawData.size(); i++) {
+            String[] parts = rawData.get(i).split(",");
+            FruitTransaction.Operation operation = FruitTransaction.Operation.fromCode(parts[0]);
+            String fruit = parts[1];
+            int quantity = Integer.parseInt(parts[2]);
+            fruitTransactions.add(new FruitTransaction(operation, fruit, quantity));
+        }


The current implementation assumes that the input data is always well-formed. Consider adding validation to handle potential parsing errors, such as checking if parts has the expected length and catching NumberFormatException when parsing the quantity.

mateacademy-ai-mentor · 2024-12-12T16:07:01Z

src/main/java/core/basesyntax/strategy/OperationStrategyImpl.java

+    }
+
+    public OperationHandler getOperationHandler(FruitTransaction.Operation operation) {
+        return operationHandlers.get(operation);


Consider adding a check to handle cases where the operationHandlers map does not contain a handler for the given operation. You might want to throw an exception or return a default handler in such cases.

mateacademy-ai-mentor · 2024-12-12T16:07:01Z

src/main/java/core/basesyntax/strategy/ShopServiceImpl.java

+            OperationHandler handler = operationStrategy
+                    .getOperationHandler(transaction.getOperation());
+            handler.handle(transaction);


Consider adding a check to handle cases where getOperationHandler might return null if a handler is not found for a given operation. This will prevent potential NullPointerException and improve robustness.

mateacademy-ai-mentor

Thank you for your hard work on this project! 🎉 Your code is well-structured and mostly meets the task requirements. However, there are a few critical issues that need attention to ensure the functionality and robustness of your code:

File Path Management: Ensure both the input (toRead.csv) and output (finalReport.csv) files are placed in the src/main/resources folder. This will help avoid file path issues and make your code more portable.
Error Handling in Data Parsing: The DataConverterImpl.java assumes that the raw data is always correctly formatted. Consider adding error handling to manage cases where the data might be malformed, such as catching ArrayIndexOutOfBoundsException or NumberFormatException.
Operation Handler Strategy: In OperationStrategyImpl.java, add a check to handle cases where an operation does not have a corresponding handler in the map. This will prevent potential NullPointerException and improve the robustness of your code.
Storage Integrity: In Storage.java, ensure the subtract method checks if the resulting quantity is negative before updating the map. Throw a RuntimeException if the quantity becomes negative to maintain data integrity.

These improvements are crucial for the functionality of your application. Please address these issues in future iterations to enhance your code quality. Keep up the great work and continue learning from these experiences! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2024-12-12T16:10:50Z

src/main/java/core/basesyntax/Main.java

+        String finalReport = reportGenerator.getReport();
+
+        MyFileWriter fileWriter = new FileWriterImpl();
+        fileWriter.write(finalReport, "src/main/resources/finalReport.csv");


The output file path should be relative and placed in the src/main/resources folder as per the checklist. Consider changing the path to src/main/resources/finalReport.csv.

mateacademy-ai-mentor · 2024-12-12T16:10:51Z

src/main/java/core/basesyntax/db/Storage.java

+    public static void subtract(String fruit, int quantity) {
+        fruits.put(fruit, fruits.getOrDefault(fruit, 0) - quantity);


The subtract method should check if the resulting quantity is negative before updating the map. If the quantity becomes negative, consider throwing a RuntimeException to prevent invalid states in the storage.

mateacademy-ai-mentor · 2024-12-12T16:10:51Z

src/main/java/core/basesyntax/service/impl/DataConverterImpl.java

+            String[] parts = rawData.get(i).split(",");
+            FruitTransaction.Operation operation = FruitTransaction.Operation.fromCode(parts[0]);
+            String fruit = parts[1];
+            int quantity = Integer.parseInt(parts[2]);


The current implementation assumes that the raw data is always correctly formatted. Consider adding error handling to manage cases where the data might be malformed, such as catching ArrayIndexOutOfBoundsException or NumberFormatException.

mateacademy-ai-mentor · 2024-12-12T16:10:51Z

src/main/java/core/basesyntax/strategy/OperationStrategyImpl.java

+    public OperationHandler getOperationHandler(FruitTransaction.Operation operation) {
+        return operationHandlers.get(operation);


Consider adding a check to handle cases where an operation does not have a corresponding handler in the map. You could throw an exception or return a default handler to prevent potential NullPointerException.

mateacademy-ai-mentor · 2024-12-12T16:10:51Z

src/main/java/core/basesyntax/strategy/ShopServiceImpl.java

+            OperationHandler handler = operationStrategy
+                    .getOperationHandler(transaction.getOperation());
+            handler.handle(transaction);


Consider adding a check to handle cases where getOperationHandler returns null. This could happen if a transaction's operation does not have a corresponding handler, leading to a NullPointerException when calling handle.

alisahoys added 2 commits December 5, 2024 13:25

Completed first draft of a SOLID task

355ff63

Completed first draft of a SOLID task

7f2f613

mateacademy-ai-mentor suggested changes Dec 5, 2024

View reviewed changes

Corrected mistakes for a SOLID task

211fe04

alisahoys requested a review from mateacademy-ai-mentor December 12, 2024 16:01

mateacademy-ai-mentor approved these changes Dec 12, 2024

View reviewed changes

Corrected mistakes for a SOLID task

3fcad69

alisahoys requested a review from mateacademy-ai-mentor December 12, 2024 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SOLID #1256

SOLID #1256

alisahoys commented Dec 5, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 5, 2024

mateacademy-ai-mentor Dec 5, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 12, 2024

mateacademy-ai-mentor Dec 12, 2024

mateacademy-ai-mentor Dec 12, 2024

mateacademy-ai-mentor Dec 12, 2024

mateacademy-ai-mentor Dec 12, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Dec 12, 2024

mateacademy-ai-mentor Dec 12, 2024

mateacademy-ai-mentor Dec 12, 2024

mateacademy-ai-mentor Dec 12, 2024

mateacademy-ai-mentor Dec 12, 2024

		public static void subtract(String fruit, int quantity) {
		fruits.put(fruit, fruits.getOrDefault(fruit, 0) - quantity);

		public OperationHandler getOperationHandler(FruitTransaction.Operation operation) {
		return operationHandlers.get(operation);

SOLID #1256

Are you sure you want to change the base?

SOLID #1256

Conversation

alisahoys commented Dec 5, 2024

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment