done #1296

xMaXD · 2025-02-24T10:24:44Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this project! 😊 There are a few critical issues that need to be addressed before we can approve the pull request. The remove method in the Storage class should update the quantity of the fruit rather than removing it from the map. It's essential to ensure that the resulting quantity is non-negative before updating the map. If the balance is negative, a RuntimeException should be thrown, as specified in the task checklist. Once these issues are resolved, your code will be in great shape! Keep up the great work! 💪

✨ 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-24T10:34:21Z

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

+public class PurchaseOperation implements OperationHandler {
+    @Override
+    public void apply(FruitTransaction transaction) {
+        Storage.remove(transaction.getFruit(), transaction.getQuantity());


Ensure that the Storage.remove method checks if the resulting quantity is non-negative before updating the storage. Throw a RuntimeException if the balance is negative, as specified in the checklist: "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 · 2025-02-24T10:34:21Z

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

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


The remove method should update the quantity of the fruit instead of removing it from the map. Use fruits.put(fruit, fruits.getOrDefault(fruit, 0) - quantity) to update the quantity. Additionally, ensure that the resulting quantity is non-negative before updating the map, and throw a RuntimeException if the balance is negative, as specified in the checklist.

mateacademy-ai-mentor

Thank you for your hard work on this project! Your code is mostly on point, but there's a critical issue that needs to be addressed before we can approve the pull request. The remove method in the Storage class is currently using Map.remove(Object key, Object value), which is not suitable for updating quantities. This could lead to incorrect stock levels. Please ensure that the method updates the quantity correctly and checks for non-negative values, throwing a RuntimeException if the balance becomes negative. Once you address this, your code will be in great shape! Keep up the great work! 🌟

✨ 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-24T10:39:41Z

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

+public class PurchaseOperation implements OperationHandler {
+    @Override
+    public void apply(FruitTransaction transaction) {
+        Storage.remove(transaction.getFruit(), transaction.getQuantity());


Ensure that the Storage.remove method checks if the resulting quantity is non-negative before updating the storage. If the balance becomes negative, throw a RuntimeException to handle this case. This is important to prevent negative stock levels in the storage.

mateacademy-ai-mentor · 2025-02-24T10:39:41Z

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

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


The remove method should update the quantity of the fruit instead of using Map.remove(Object key, Object value). Ensure that the quantity does not become negative. If the resulting quantity is negative, throw a RuntimeException to handle this case.

mateacademy-ai-mentor

Great job on implementing the solution! 🎉 Your code demonstrates a solid understanding of the task requirements and effectively addresses the main functionality. However, there are a few areas for improvement:

Package Structure: Ensure your classes are organized into the recommended package structure, such as placing the Storage class in a db package.
SOLID Principles: Consider refactoring the Storage class to adhere to the Dependency Inversion Principle by avoiding direct static access.
Encapsulation: Make the fruits map in the Storage class private and provide access through methods to maintain encapsulation.
Exception Handling: Improve the exception messages in the PurchaseOperation class to provide more informative feedback.
File Management: Ensure that files for reading from and writing to CSV files are included in the src/main/resources folder.

These improvements will enhance the maintainability and robustness of your code. Remember, addressing these issues will not only help you in this project but also in future endeavors. Keep up the good work, and continue learning from each experience! 🚀

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

mateacademy-ai-mentor

Review Summary

Great job on implementing the BalanceOperation, DataConverter, and other classes! Your code demonstrates a solid understanding of interfaces and the strategy pattern. 🎉

However, there are a few areas that need improvement:

Exception Handling: In the PurchaseOperation class, the exception handling could be more informative. Consider providing more context in the exception message, such as including the fruit type and the attempted quantity, to make debugging easier.
Package Structure: The project would benefit from a more organized package structure. Consider separating your classes into packages like db, model, service, service.impl, and strategy.
SOLID Principles: Ensure that all services are independent and adhere to SOLID principles. The main() method should orchestrate the flow by calling independent services.
File Placement: Input and output files should ideally be placed in the src/main/resources folder to maintain a clean project structure.
Avoid Hardcoding: Try to avoid hardcoding file paths and other values outside of the Main class.

The decision to approve is made to help you move forward with your learning. Please review the comments and address these issues in future iterations to enhance the quality and maintainability of your code. Keep up the good work and continue to learn from these experiences! 🚀

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

mateacademy-ai-mentor · 2025-02-24T11:03:32Z

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

+        } catch (RuntimeException e) {
+            throw new RuntimeException("Balance is negative");


Consider providing more context in the exception message, such as including the fruit type and the attempted quantity, to make debugging easier.

Elena-Bruyako · 2025-02-24T21:07:10Z

src/main/java/core/basesyntax/BalanceOperation.java

@@ -0,0 +1,8 @@
+package core.basesyntax;


Let's revise common mistakes file and check project structure

Elena-Bruyako · 2025-02-24T21:07:20Z

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

+class DataConverterImpl implements DataConverter {
+    @Override
+    public List<FruitTransaction> convertToTransaction(List<String> data) {
+        // Пропускаем первую строку с заголовками


remove comment

Elena-Bruyako · 2025-02-24T21:07:37Z

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

+                    int quantity = Integer.parseInt(parts[2]);
+                    return new FruitTransaction(op, fruit, quantity);
+                })
+                .collect(Collectors.toList());


Suggested change

.collect(Collectors.toList());

.toList();

Elena-Bruyako · 2025-02-24T21:07:46Z

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

+        return data.stream()
+                .skip(1)
+                .map(line -> {
+                    String[] parts = line.split(",");


create constant for comma

Elena-Bruyako · 2025-02-24T21:07:56Z

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

+                .skip(1)
+                .map(line -> {
+                    String[] parts = line.split(",");
+                    String opCode = parts[0];


create constants for all magic numbers

Elena-Bruyako · 2025-02-24T21:09:45Z

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

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


input and output files should be in resources folder
create constants for files pathes

Elena-Bruyako · 2025-02-24T21:10:10Z

src/main/java/core/basesyntax/ReportGeneratorImpl.java

+    public String getReport() {
+        StringBuilder report = new StringBuilder();
+        Storage.getFruits().forEach((fruit, quantity) -> report.append(fruit)
+                        .append(",")


create constant for comma

Elena-Bruyako · 2025-02-24T21:10:35Z

src/main/java/core/basesyntax/ReportGeneratorImpl.java

+        Storage.getFruits().forEach((fruit, quantity) -> report.append(fruit)
+                        .append(",")
+                        .append(quantity)
+                        .append("\n"));


Use System.lineSeparator() instead of "\n"

Elena-Bruyako · 2025-02-24T21:10:46Z

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

+
+import java.util.List;
+
+class ShopServiceImpl implements ShopService {


Suggested change

class ShopServiceImpl implements ShopService {

public class ShopServiceImpl implements ShopService {

Elena-Bruyako · 2025-02-24T21:11:04Z

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

+        }
+    }
+
+    public static Map<String, Integer> getFruits() {


redundant method

Elena-Bruyako · 2025-02-27T21:54:13Z

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

+import java.util.Map;
+
+public class Main {
+    private static final String REPORT_TO_READ_FILE_PATH = "reportToRead.csv";


files should be placed in resources folder

Elena-Bruyako · 2025-02-27T21:55:27Z

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

+
+        public String getCode() {
+            return code;
+        }


You can also check if particular code exists in public static Operation getOperation(String code)

Elena-Bruyako · 2025-02-27T21:59:33Z

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

+                    String[] parts = line.split(SEPARATOR_SIGN);
+                    String opCode = parts[OPERATOR_PART];
+                    FruitTransaction.Operation op = switch (opCode) {
+                        case "b" -> FruitTransaction.Operation.BALANCE;


let's use getOperation(code) method instead of switch case
See comment in FruitTransaction

done

271b442

mateacademy-ai-mentor suggested changes Feb 24, 2025

View reviewed changes

fix1

821e1c2

xMaXD requested a review from mateacademy-ai-mentor February 24, 2025 10:50

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

View reviewed changes

Elena-Bruyako suggested changes Feb 24, 2025

View reviewed changes

fix2

b7a2290

xMaXD requested a review from Elena-Bruyako February 27, 2025 16:11

Elena-Bruyako suggested changes Feb 27, 2025

View reviewed changes

fix3

2465e8e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

done #1296

done #1296

xMaXD commented Feb 24, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Feb 24, 2025

mateacademy-ai-mentor Feb 24, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Feb 24, 2025

mateacademy-ai-mentor Feb 24, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 24, 2025

Elena-Bruyako Feb 27, 2025

Elena-Bruyako Feb 27, 2025

Elena-Bruyako Feb 27, 2025

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

		} catch (RuntimeException e) {
		throw new RuntimeException("Balance is negative");


		import java.util.List;

		class ShopServiceImpl implements ShopService {

	class ShopServiceImpl implements ShopService {
	public class ShopServiceImpl implements ShopService {

done #1296

Are you sure you want to change the base?

done #1296

Conversation

xMaXD commented Feb 24, 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

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

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

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

Choose a reason for hiding this comment