jv-1-fruit-shop #1285

aunkerrr · 2025-01-26T18:17:42Z

No description provided.

mateacademy-ai-mentor

Great job on implementing the solution! 🎉 Your code adheres to the SOLID principles and meets the task requirements effectively. While there are minor suggestions, such as moving file paths to a configuration file to avoid hardcoding, these do not impact the core functionality. Keep up the excellent work and consider these improvements for future projects! 😊

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

mateacademy-ai-mentor · 2025-01-26T18:21:04Z

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

+            List<String> inputLines = fileReader.read("C:/Users/WeakRitta/IdeaProjects"
+                    + "/jv-fruit-shop/src/main/java/result/reportToRead.csv");


Consider moving the file paths to a configuration file or using a resource folder to avoid hardcoding paths directly in the code. This aligns with the checklist recommendation to avoid hardcoding in your solution.

mateacademy-ai-mentor · 2025-01-26T18:21:04Z

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

+            fileWriter.write(report, "C:/Users/WeakRitta/IdeaProjects/jv-fruit-shop/src"
+                    + "/main/java/result/finalReport.csv");


Similarly, consider moving the output file path to a configuration file or using a resource folder to avoid hardcoding paths directly in the code.

Elena-Bruyako · 2025-01-26T20:33:55Z

README.md

 ```java
 public class Main {
    public static void main(String[] arg) {
        // 1. Read the data from the input CSV file
        FileReader fileReader = new FileReaderImpl();
-        List<String> inputReport = fileReader.read("reportToRead.csv");
+        List<String> inputReport = fileReader.read("result/reportToRead.csv");


revert changes

Elena-Bruyako · 2025-01-26T20:34:45Z

pom.xml

@@ -18,7 +18,14 @@
    </properties>

    <dependencies>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>


you should implement tests in the next task

Elena-Bruyako · 2025-01-26T20:36:12Z

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

+            ReportGenerator reportGenerator = new ReportGeneratorImpl();
+            FileWriter fileWriter = new FileWriterImpl();
+
+            List<String> inputLines = fileReader.read("C:/Users/WeakRitta/IdeaProjects"


create constant for path, also files should be in resources folder
don't use absolute path

Elena-Bruyako · 2025-01-26T20:36:19Z

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

+            List<FruitTransaction> transactions = dataConverter.convertToTransactions(inputLines);
+            Map<String, Integer> finalStorage = shopService.process(transactions);
+            String report = reportGenerator.generateReport(finalStorage);
+            fileWriter.write(report, "C:/Users/WeakRitta/IdeaProjects/jv-fruit-shop/src"


same comment

Elena-Bruyako · 2025-01-26T20:36:37Z

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

+
+public class Main {
+    public static void main(String[] args) {
+        try {


remove try catch

Elena-Bruyako · 2025-01-26T20:38:55Z

src/main/java/result/ReportGeneratorImpl.java

+                        .append(entry.getKey())
+                        .append(",")
+                        .append(entry.getValue())
+                        .append("\n"));


Elena-Bruyako · 2025-01-26T20:39:05Z

src/main/java/result/ReportGeneratorImpl.java

+                .sorted(Map.Entry.comparingByKey())
+                .forEach(entry -> report
+                        .append(entry.getKey())
+                        .append(",")


create constant for comma

Elena-Bruyako · 2025-01-26T20:39:21Z

src/test/java/core/basesyntax/DataConverterImplTest.java

+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class DataConverterImplTest {


remove all tests

Elena-Bruyako · 2025-01-26T20:39:51Z

src/main/java/result/ReportGeneratorImpl.java

+    @Override
+    public String generateReport(Map<String, Integer> storage) {
+        StringBuilder report = new StringBuilder();
+        report.append("fruit,quantity\n");


you can create constant HEADER = "fruit,quantity";

Elena-Bruyako · 2025-01-26T20:40:12Z

src/main/java/core/basesyntax/converter/DataConverterImpl.java

+        List<FruitTransaction> transactions = new ArrayList<>();
+        // Skip header
+        for (int i = 1; i < lines.size(); i++) {
+            String[] parts = lines.get(i).split(",");


create constant for comma

liliia-ponomarenko

Good job! Let’s improve your solution ;)

liliia-ponomarenko · 2025-01-27T07:58:49Z

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

+    private static final String READ_PATH = "reportToRead.csv";
+    private static final String WRITE_PATH = "finalReport.csv";
+


place files in the resources directory

liliia-ponomarenko · 2025-01-27T07:59:19Z

src/main/java/core/basesyntax/converter/DataConverterImpl.java

+            String operationCode = parts[0].trim();
+            String fruit = parts[1].trim();
+            int quantity;
+            try {
+                quantity = Integer.parseInt(parts[2].trim());


Suggested change

String operationCode = parts[0].trim();

String fruit = parts[1].trim();

int quantity;

try {

quantity = Integer.parseInt(parts[2].trim());

String operationCode = parts[0];

String fruit = parts[1].trim();

int quantity;

try {

quantity = Integer.parseInt(parts[2]);

liliia-ponomarenko · 2025-01-27T07:59:51Z

src/main/java/core/basesyntax/io/FileReaderImpl.java

+public class FileReaderImpl implements FileReader {
+    @Override
+    public List<String> read(String filePath) throws IOException {
+        if (filePath == null || filePath.trim().isEmpty()) {


Suggested change

if (filePath == null || filePath.trim().isEmpty()) {

if (filePath == null || filePath.isEmpty()) {

liliia-ponomarenko · 2025-01-27T08:00:18Z

src/main/java/core/basesyntax/io/FileWriterImpl.java

+        try (BufferedWriter writer = new BufferedWriter(new java.io.FileWriter(filePath))) {
+            writer.write(content);
+        } catch (IOException e) {
+            System.out.println("An error occurred while writing to the file: " + e.getMessage());


throw exception

liliia-ponomarenko · 2025-01-27T08:00:40Z

src/main/java/core/basesyntax/model/FruitTransaction.java

+        }
+    }
+}
+


Suggested change

leave only one empty line

liliia-ponomarenko · 2025-01-27T08:04:20Z

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

+        if (handler == null) {
+            throw new IllegalArgumentException("No handler found" + transaction.getOperation());
+        }
+        handler.handle(storage, transaction);


The strategy should return the relevant handler. for process data use a separate ShopService.

Suggested change

handler.handle(storage, transaction);

return handler;

liliia-ponomarenko · 2025-01-27T08:05:26Z

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

+
+public class ShopServiceImpl implements ShopService {
+    private final OperationStrategy operationStrategy;
+    private final Map<String, Integer> storage = new HashMap<>();


remove this variable. Use static map from the Storage class

liliia-ponomarenko · 2025-01-27T08:05:55Z

src/main/java/report/ReportGeneratorImpl.java

+
+        storage.entrySet()
+                .stream()
+                .sorted(Map.Entry.comparingByKey())


It’s not required

liliia-ponomarenko · 2025-01-27T08:06:16Z

src/main/java/report/ReportGeneratorImpl.java

+                .sorted(Map.Entry.comparingByKey())
+                .forEach(entry -> report
+                        .append(entry.getKey())
+                        .append(",")


move the separator to constant fields

liliia-ponomarenko · 2025-01-27T08:06:21Z

src/main/java/report/ReportGeneratorImpl.java

+                        .append(",")
+                        .append(entry.getValue())
+                        .append(System.lineSeparator()));
+


Suggested change

liliia-ponomarenko

Good job! Let’s fix a few mistakes ;)

liliia-ponomarenko · 2025-01-28T07:52:30Z

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

+    private static final String READ_PATH = "recources/reportToRead.csv";
+    private static final String WRITE_PATH = "recources/finalReport.csv";


use the system resources directory. the path should be src/main/resources/reportToRead.csv

liliia-ponomarenko · 2025-01-28T07:52:54Z

src/main/java/core/basesyntax/converter/DataConverterImpl.java

+            }
+
+            String operationCode = parts[0];
+            String fruit = parts[1].trim();


Suggested change

String fruit = parts[1].trim();

String fruit = parts[1];

liliia-ponomarenko · 2025-01-28T07:53:50Z

src/main/java/core/basesyntax/io/FileReaderImpl.java

+
+public class FileReaderImpl implements FileReader {
+    @Override
+    public List<String> read(String filePath) throws IOException {


Don’t throw exceptions in the method signature. add catch block

Not resolved

liliia-ponomarenko · 2025-01-28T07:54:43Z

src/main/java/core/basesyntax/io/FileWriterImpl.java

+        try (BufferedWriter writer = new BufferedWriter(new java.io.FileWriter(filePath))) {
+            writer.write(content);
+        } catch (IOException e) {
+            throw new IOException("An error occurred while writing to the file: " + e.getMessage());


Suggested change

throw new IOException("An error occurred while writing to the file: " + e.getMessage());

throw new RuntimeException("An error occurred while writing to the file: " + filePath, e);

liliia-ponomarenko · 2025-01-28T07:55:46Z

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

+    public Map<String, Integer> process(List<FruitTransaction> transactions) {
+        for (FruitTransaction transaction : transactions) {
+            OperationHandler handler = operationStrategy.getHandler(transaction.getOperation());
+            handler.handle(transaction);
+        }
+        return new HashMap<>(Storage.storage);


Suggested change

public Map<String, Integer> process(List<FruitTransaction> transactions) {

for (FruitTransaction transaction : transactions) {

OperationHandler handler = operationStrategy.getHandler(transaction.getOperation());

handler.handle(transaction);

}

return new HashMap<>(Storage.storage);

public void process(List<FruitTransaction> transactions) {

for (FruitTransaction transaction : transactions) {

OperationHandler handler = operationStrategy.getHandler(transaction.getOperation());

handler.handle(transaction);

}

liliia-ponomarenko · 2025-01-28T07:56:18Z

src/main/java/report/ReportGeneratorImpl.java

+    @Override
+    public String generateReport(Map<String, Integer> storage) {
+        StringBuilder report = new StringBuilder();
+        report.append(HEADER).append(System.lineSeparator());
+
+        storage.entrySet()


Suggested change

@Override

public String generateReport(Map<String, Integer> storage) {

StringBuilder report = new StringBuilder();

report.append(HEADER).append(System.lineSeparator());

storage.entrySet()

@Override

public String generateReport() {

StringBuilder report = new StringBuilder();

report.append(HEADER).append(System.lineSeparator());

Storage.storage.entrySet()

ahoienko

Nice, left some comments

ahoienko · 2025-01-29T11:29:31Z

src/main/java/core/basesyntax/converter/DataConverterImpl.java

+            String operationCode = parts[0];
+            String fruit = parts[1];


Magic numbers

ahoienko · 2025-01-29T11:29:45Z

src/main/java/core/basesyntax/converter/DataConverterImpl.java

+            try {
+                quantity = Integer.parseInt(parts[2]);
+                if (quantity < 0) {
+                    throw new IllegalArgumentException("Quantity cannot be negative at line "
+                            + (i + 1));
+                }
+            } catch (NumberFormatException e) {
+                throw new IllegalArgumentException("Invalid quantity format at line " + (i + 1));
+            }


Move to separate method

ahoienko · 2025-01-29T11:30:25Z

src/main/java/core/basesyntax/converter/DataConverterImpl.java

+        for (int i = 1; i < lines.size(); i++) {
+            String[] parts = lines.get(i).split(COMMA);
+            if (parts.length != 3) {


Magic numbers

ahoienko · 2025-01-29T11:30:40Z

src/main/java/core/basesyntax/io/FileReaderImpl.java

+
+public class FileReaderImpl implements FileReader {
+    @Override
+    public List<String> read(String filePath) throws IOException {


Not resolved

ahoienko · 2025-01-29T11:31:22Z

src/main/java/core/basesyntax/io/FileReaderImpl.java

+                lines.add(line);
+            }
+        } catch (IOException e) {
+            throw new IOException("An error occurred while readint this file: " + e);


Suggested change

throw new IOException("An error occurred while readint this file: " + e);

throw new SomeRutimeException("An error occurred while readint this file: " + e);

ahoienko · 2025-01-29T11:31:41Z

src/main/java/core/basesyntax/io/FileReaderImpl.java

+                lines.add(line);
+            }
+        } catch (IOException e) {
+            throw new IOException("An error occurred while readint this file: " + e);


Which file? What is 'this' file?

ahoienko · 2025-01-29T11:31:54Z

src/main/java/core/basesyntax/io/FileWriter.java

+import java.io.IOException;
+
+public interface FileWriter {
+    void write(String content, String filePath) throws IOException;


Suggested change

void write(String content, String filePath) throws IOException;

void write(String content, String filePath)

Not resolved

ahoienko · 2025-01-29T11:32:48Z

src/main/java/report/ReportGeneratorImpl.java

+    public String generateReport() { // Remove the Map parameter
+        StringBuilder report = new StringBuilder();
+        report.append(HEADER).append(System.lineSeparator());
+
+        Storage.storage.entrySet() // Use Storage.storage directly


remove comments

ahoienko · 2025-01-29T11:32:53Z

src/main/java/report/ReportGeneratorImpl.java

@@ -0,0 +1,23 @@
+package report;
+
+import core.basesyntax.operation.Storage; // Import the Storage class


remove comments

ahoienko · 2025-01-29T11:33:20Z

README.md

 Also, here is an example of what the `main` method may look like:
+
 ```java
 public class Main {
    public static void main(String[] arg) {
        // 1. Read the data from the input CSV file
        FileReader fileReader = new FileReaderImpl();
-        List<String> inputReport = fileReader.read("reportToRead.csv");
+       List<String> inputReport = fileReader.read("reportToRead.csv");

        // 2. Convert the incoming data into FruitTransactions list
        DataConverter dataConverter = new DataConverterImpl();


Are you sure that you need this changes?

Kinda sure cause i just suddenly changed task description and mentor noticed this before
So yeah im pretty sure

No, you actually dont, please dont change readme file, it is not related to task, and changes that you made are redundant

ahoienko

Please be careful and read comments carefully, and push new solution after you carefully checked all comments))

ahoienko · 2025-01-30T12:10:24Z

src/main/java/core/basesyntax/converter/DataConverterImpl.java

+        String operationCode = parts[0];
+        String fruit = parts[1];
+        int quantity = parseQuantity(parts[2], lineNumber);


ahoienko · 2025-01-30T12:11:08Z

src/main/java/core/basesyntax/io/FileReader.java

+import java.util.List;
+
+public interface FileReader {
+    List<String> read(String filePath) throws IOException;


Please be careful, if throws is removed from implementation it should be removed from interface as well

ahoienko · 2025-01-30T12:12:54Z

src/main/java/core/basesyntax/io/FileWriter.java

+import java.io.IOException;
+
+public interface FileWriter {
+    void write(String content, String filePath) throws IOException;


Not resolved

ahoienko · 2025-01-30T12:15:00Z

README.md

 Also, here is an example of what the `main` method may look like:
+
 ```java
 public class Main {
    public static void main(String[] arg) {
        // 1. Read the data from the input CSV file
        FileReader fileReader = new FileReaderImpl();
-        List<String> inputReport = fileReader.read("reportToRead.csv");
+       List<String> inputReport = fileReader.read("reportToRead.csv");

        // 2. Convert the incoming data into FruitTransactions list
        DataConverter dataConverter = new DataConverterImpl();


No, you actually dont, please dont change readme file, it is not related to task, and changes that you made are redundant

aunkerrr · 2025-01-30T21:49:26Z

Please be careful and read comments carefully, and push new solution after you carefully checked all comments))

okay thanks a lot for your review
hope i did it well

Elena-Bruyako · 2025-02-03T21:48:52Z

src/main/java/core/basesyntax/resorces/finalReport.csv

@@ -0,0 +1,6 @@
+fruit,quantity


resources folder should be in main folder

jv-1-fruit-shop

8c59772

mateacademy-ai-mentor approved these changes Jan 26, 2025

View reviewed changes

aunkerrr added 2 commits January 26, 2025 19:34

jv-2-fruit-shop-minor-changes

5bfaf47

jv-2-fruit-shop-tests-added

48aac1f

Elena-Bruyako suggested changes Jan 26, 2025

View reviewed changes

jv-3-fruit-shop-minor-changes

1e9bc33

aunkerrr requested a review from Elena-Bruyako January 27, 2025 00:06

liliia-ponomarenko suggested changes Jan 27, 2025

View reviewed changes

jv-4-fruit-shop-corrections

36b602c

aunkerrr requested a review from liliia-ponomarenko January 27, 2025 21:31

jv-4-fruit-shop-corrections

25aed10

liliia-ponomarenko suggested changes Jan 28, 2025

View reviewed changes

jv-4-fruit-shop-corrections

74b4b18

aunkerrr requested review from mateacademy-ai-mentor and liliia-ponomarenko January 28, 2025 20:11

ahoienko suggested changes Jan 29, 2025

View reviewed changes

jv-5-fruit-shop-corrections

0733363

aunkerrr requested a review from ahoienko January 29, 2025 16:53

ahoienko suggested changes Jan 30, 2025

View reviewed changes

jv-6-fruit-shop-corrections

4acd321

aunkerrr requested a review from ahoienko February 3, 2025 18:50

Elena-Bruyako suggested changes Feb 3, 2025

View reviewed changes

src/main/java/core/basesyntax/resorces/finalReport.csv Outdated

@@ -0,0 +1,6 @@

fruit,quantity

Copy link

Elena-Bruyako Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resources folder should be in main folder

jv-7-fruit-shop-corrections

97eb82c

aunkerrr requested a review from Elena-Bruyako February 10, 2025 21:50

Elena-Bruyako approved these changes Feb 11, 2025

View reviewed changes

		List<String> inputLines = fileReader.read("C:/Users/WeakRitta/IdeaProjects"
		+ "/jv-fruit-shop/src/main/java/result/reportToRead.csv");

		fileWriter.write(report, "C:/Users/WeakRitta/IdeaProjects/jv-fruit-shop/src"
		+ "/main/java/result/finalReport.csv");

		private static final String READ_PATH = "reportToRead.csv";
		private static final String WRITE_PATH = "finalReport.csv";

	if (filePath == null \|\| filePath.trim().isEmpty()) {
	if (filePath == null \|\| filePath.isEmpty()) {

	throw new IOException("An error occurred while writing to the file: " + e.getMessage());
	throw new RuntimeException("An error occurred while writing to the file: " + filePath, e);

	throw new IOException("An error occurred while readint this file: " + e);
	throw new SomeRutimeException("An error occurred while readint this file: " + e);

	void write(String content, String filePath) throws IOException;
	void write(String content, String filePath)

		@@ -0,0 +1,23 @@
		package report;

		import core.basesyntax.operation.Storage; // Import the Storage class

jv-1-fruit-shop #1285

Are you sure you want to change the base?

jv-1-fruit-shop #1285

Conversation

aunkerrr commented Jan 26, 2025

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

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

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

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

ahoienko 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aunkerrr Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahoienko 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

aunkerrr commented Jan 30, 2025

Choose a reason for hiding this comment

aunkerrr Jan 29, 2025 •

edited

Loading