implemented fruit shop program logic using solid principles #1292

kriznychenko · 2025-02-12T10:28:29Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this project! While your code demonstrates a solid understanding of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

Method Naming: The method getOperation in the OperationHandler interface and its implementations is misleading. It should be renamed to something like execute or performOperation to better reflect its purpose.
Resource Management: Ensure that file handling in FileReaderImpl and FileWriterImpl uses try-with-resources to guarantee proper closure of resources and prevent potential leaks.
Error Handling: Improve exception handling by providing more informative messages, especially when parsing integers or handling file operations. This will make debugging easier and improve user experience.
Data Encapsulation: The Storage class should not expose its internal map directly. Return a copy of the map to prevent unintended modifications from outside the class.
Constructor Redundancy: The constructor in FileWriterImpl is redundant. Consider removing it unless you plan to extend its functionality.
File Path Handling: Avoid using hardcoded file paths. Use relative paths to ensure the code is portable across different environments.

By addressing these issues, you'll enhance the code's robustness and maintainability. Keep up the great work, and I'm looking forward to seeing the improvements! 😊

✨ 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 · 2025-02-12T10:32:35Z

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

+
+public class Main {
+    public static void main(String[] args) {
+        FileReader fileReader = new FileReaderImpl("src/main/java/db/reportToRead.csv");


The FileReaderImpl constructor is being called with a hardcoded file path. Consider using a relative path to ensure portability across different environments .

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

src/main/java/service/operation/BalanceOperation.java

src/main/java/service/operation/OperationHandler.java

src/main/java/service/operation/PurchaseOperation.java

src/main/java/service/operation/ReturnOperation.java

src/main/java/service/operation/SupplyOperation.java

mateacademy-ai-mentor

Your Pull Request has been approved! 🎉

While your code meets the task requirements, there are several areas for improvement that can enhance the robustness and maintainability of your code:

Exception Handling: Ensure that exception messages are informative and provide context, such as row numbers or file names, to aid in debugging. Consider logging errors or adding comments to explain exceptions, especially for unknown operation codes.
Resource Management: Use try-with-resources for both FileReader and FileWriter to ensure proper closure and prevent resource leaks.
Separation of Concerns: The parseCsvRow method should be moved to a class that handles data conversion, such as DataConverterImpl, to improve separation of concerns.
Data Encapsulation: Avoid direct instantiation of Storage to maintain encapsulation. Use methods to access its data instead.
Null Handling: Add checks for potential null returns from methods like operationStrategy.get to prevent NullPointerException.
Constructor Redundancy: Remove redundant constructors unless additional functionality is planned.

Your efforts are commendable, and addressing these issues will help you grow as a developer. 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! ✨

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

mateacademy-ai-mentor · 2025-02-12T13:28:16Z

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

+            FileWriter fileWriter = new FileWriterImpl("finalReport.csv");
+            fileWriter.write(resultingReport);
+        } catch (IOException e) {
+            throw new RuntimeException("Enable to create FileWriter for finalReport.csv", e);


The exception message here could be more informative. Consider changing it to something like 'Unable to write to finalReport.csv'.

mateacademy-ai-mentor · 2025-02-12T13:28:16Z

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

+        ShopService shopService = new ShopServiceImpl(operationStrategy);
+        shopService.process(transactions);
+
+        Storage storage = new Storage();


Direct instantiation of Storage might expose its internal state. Consider using a method to access its data to maintain encapsulation.

mateacademy-ai-mentor · 2025-02-12T13:28:16Z

src/main/java/service/impl/FileReaderImpl.java

+    public FruitTransaction parseCsvRow(String csvRow) {
+        String[] fields = csvRow.split(",");
+        FruitTransaction fruitTransaction = new FruitTransaction();
+        fruitTransaction.setOperation(FruitTransaction.Operation.valueOf(fields[0]));
+        fruitTransaction.setFruit(fields[1]);
+        try {
+            fruitTransaction.setQuantity(Integer.parseInt(fields[2]));
+        } catch (NumberFormatException e) {
+            throw new IllegalArgumentException("Invalid quantity format: " + fields[2], e);
+        }
+        return fruitTransaction;


The parseCsvRow method is responsible for parsing CSV rows into FruitTransaction objects, which is not directly related to file reading. Consider moving this method to a class that handles data conversion, such as DataConverterImpl, to improve separation of concerns.

mateacademy-ai-mentor · 2025-02-12T13:28:17Z

src/main/java/service/impl/FileReaderImpl.java

+        try {
+            fruitTransaction.setQuantity(Integer.parseInt(fields[2]));
+        } catch (NumberFormatException e) {
+            throw new IllegalArgumentException("Invalid quantity format: " + fields[2], e);


The error message for Invalid quantity format is informative, but consider including the row number or more context to aid in debugging when this exception is thrown.

mateacademy-ai-mentor · 2025-02-12T13:28:17Z

src/main/java/service/impl/FileWriterImpl.java

+    public FileWriterImpl(String fileName) throws IOException {
+        super(fileName);
+    }


The constructor in FileWriterImpl is redundant as it does not add any new functionality beyond calling the superclass constructor. Consider removing it unless you plan to add additional functionality.

mateacademy-ai-mentor · 2025-02-12T13:28:17Z

src/main/java/service/impl/OperationStrategyImpl.java

+
+    @Override
+    public OperationHandler get(FruitTransaction.Operation operationType) {
+        return operationHandlerMap.get(operationType);


The get method may return null if the operation type is not found in the map. Consider handling this case by returning a default handler, throwing an exception, or providing a meaningful error message to avoid potential NullPointerException.

mateacademy-ai-mentor · 2025-02-12T13:28:17Z

src/main/java/service/impl/ShopServiceImpl.java

+            OperationHandler handler = operationStrategy.get(transaction.getOperation());
+            handler.performOperation(transaction);


The operationStrategy.get method may return null if no handler is found for a given operation. Consider adding a null check and handling this case to prevent potential NullPointerException.

liliia-ponomarenko

Good job! Let’s improve your solution ;)

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

liliia-ponomarenko · 2025-02-13T09:59:24Z

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

+        try {
+            FileWriter fileWriter = new FileWriterImpl("finalReport.csv");
+            fileWriter.write(resultingReport);
+        } catch (IOException e) {
+            throw new RuntimeException("Unable to create FileWriter for finalReport.csv", e);
+        }


src/main/java/db/reportToRead.csv

src/main/java/model/FruitTransaction.java

src/main/java/service/impl/FileReaderImpl.java

liliia-ponomarenko · 2025-02-13T10:06:19Z

src/main/java/service/impl/FileWriterImpl.java

+    public FileWriterImpl(String fileName) throws IOException {
+        super(fileName);
+    }


liliia-ponomarenko · 2025-02-13T10:06:38Z

src/main/java/service/impl/FileWriterImpl.java

+import java.io.FileWriter;
+import java.io.IOException;
+
+public class FileWriterImpl extends FileWriter {


add writing logic

src/main/java/service/impl/ReportGeneratorImpl.java

kriznychenko · 2025-02-13T11:26:48Z

Everything was understandable and clear.
Thank you with best wishes!

liliia-ponomarenko

Good job! A few minor comments. But fix it before testing your solution ;)

liliia-ponomarenko · 2025-02-14T07:56:00Z

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

+import strategy.OperationStrategy;
+
+public class Main {
+    private static final String REPORT_TO_READ_FILE = "src/main/java/resources/reportToRead.csv";


Create a system resources directory. It has an incorrect level

Suggested change

private static final String REPORT_TO_READ_FILE = "src/main/java/resources/reportToRead.csv";

private static final String REPORT_TO_READ_FILE = "src/main/resources/reportToRead.csv";

liliia-ponomarenko · 2025-02-14T07:56:43Z

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

+        return fruitTransaction;
+    }
+}
+


Suggested change

leave only one empty line

liliia-ponomarenko · 2025-02-14T07:56:54Z

src/main/java/service/impl/FileReaderImpl.java

+    public FileReaderImpl() {
+
+    }


Suggested change

public FileReaderImpl() {

}

fix comment

liliia-ponomarenko · 2025-02-14T07:57:59Z

src/main/java/service/impl/FileReaderImpl.java

+        }
+        try {
+            List<String> lines = Files.readAllLines(file.toPath());
+            return lines.subList(1, lines.size());


move header skipping to DataConverter. This method should only read file content

Elena-Bruyako · 2025-02-17T22:47:59Z

src/main/java/db/Storage.java

+    public Map<String, Integer> getAllFruits() {
+        return new HashMap<>(fruits);


fix comment

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

Elena-Bruyako · 2025-02-17T22:48:34Z

src/main/java/service/impl/FileReaderImpl.java

+    public FileReaderImpl() {
+
+    }


fix comment

Elena-Bruyako · 2025-02-17T22:49:02Z

src/main/java/service/impl/FileWriterImpl.java

+
+public class FileWriterImpl {
+    public void write(String filePath, String content) {
+        try (FileWriter fileWriter = new FileWriter(filePath)) {


check if FileWriter implements AutoCloseable

src/main/java/service/impl/ReportGeneratorImpl.java

implemented fruit shop program logic using solid principles

9c9e0b4

mateacademy-ai-mentor suggested changes Feb 12, 2025

View reviewed changes

KaterynaRiznychenko added 2 commits February 12, 2025 12:39

replaced imports due to mvn

dac23fa

corrected code thanks to ai-buddy

10ebf10

kriznychenko requested a review from mateacademy-ai-mentor February 12, 2025 13:24

mateacademy-ai-mentor approved these changes Feb 12, 2025

View reviewed changes

corrected code thanks to ai-buddy

e830cb2

liliia-ponomarenko suggested changes Feb 13, 2025

View reviewed changes

KaterynaRiznychenko added 7 commits February 13, 2025 12:35

moved files path into constant fields

a12a686

moved files into resources package

032eb85

moved nested class after methods of the top-level class

49e39f5

moved the separator to constant fields

ee9f43e

improved methods of FileReaderImpl class

142296e

remade writing logic for the final report

71a9138

improved checkstyle ny maven

c19eee1

kriznychenko requested a review from liliia-ponomarenko February 13, 2025 11:26

liliia-ponomarenko suggested changes Feb 14, 2025

View reviewed changes

KaterynaRiznychenko added 3 commits February 17, 2025 12:52

improved code

b0dd1dd

improved code

06aa862

improved code

b977aa7

kriznychenko requested a review from liliia-ponomarenko February 17, 2025 10:55

Elena-Bruyako suggested changes Feb 17, 2025

View reviewed changes

KaterynaRiznychenko added 2 commits February 21, 2025 15:02

removed magic numbers

dfec118

made some changes

31644a0

kriznychenko requested a review from Elena-Bruyako February 21, 2025 13:15

Elena-Bruyako approved these changes Feb 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implemented fruit shop program logic using solid principles #1292

implemented fruit shop program logic using solid principles #1292

kriznychenko commented Feb 12, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Feb 12, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Feb 12, 2025

mateacademy-ai-mentor Feb 12, 2025

mateacademy-ai-mentor Feb 12, 2025

mateacademy-ai-mentor Feb 12, 2025

mateacademy-ai-mentor Feb 12, 2025

mateacademy-ai-mentor Feb 12, 2025

mateacademy-ai-mentor Feb 12, 2025

liliia-ponomarenko left a comment

liliia-ponomarenko Feb 13, 2025

liliia-ponomarenko Feb 13, 2025

liliia-ponomarenko Feb 13, 2025

kriznychenko commented Feb 13, 2025

liliia-ponomarenko left a comment

liliia-ponomarenko Feb 14, 2025

liliia-ponomarenko Feb 14, 2025

liliia-ponomarenko Feb 14, 2025

Elena-Bruyako Feb 17, 2025

liliia-ponomarenko Feb 14, 2025

Elena-Bruyako Feb 17, 2025

Elena-Bruyako Feb 17, 2025

Elena-Bruyako Feb 17, 2025

		OperationHandler handler = operationStrategy.get(transaction.getOperation());
		handler.performOperation(transaction);

	private static final String REPORT_TO_READ_FILE = "src/main/java/resources/reportToRead.csv";
	private static final String REPORT_TO_READ_FILE = "src/main/resources/reportToRead.csv";

		public Map<String, Integer> getAllFruits() {
		return new HashMap<>(fruits);

implemented fruit shop program logic using solid principles #1292

Are you sure you want to change the base?

implemented fruit shop program logic using solid principles #1292

Conversation

kriznychenko commented Feb 12, 2025

mateacademy-ai-mentor left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liliia-ponomarenko 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

kriznychenko commented Feb 13, 2025

liliia-ponomarenko 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment