first version #1235

MateuszSeler · 2024-11-05T18:32:21Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this project! 😊 While your code is on the right track, there are a few critical issues that need to be addressed before we can approve the pull request:

Exception Handling: When rethrowing exceptions, it's important to provide informative messages. This will help in debugging and understanding the source of errors more easily.
Resource Management: Implementing try-with-resources for BufferedReader and BufferedWriter is essential to prevent resource leaks and ensure that resources are closed properly, even in the event of an exception.
Operation Handling: Ensure that your code gracefully handles cases where an operation symbol is not found in the map. Consider throwing an exception or providing a default operation.
Data Validation: Add validation checks for CSV data to handle potential exceptions like ArrayIndexOutOfBoundsException or NumberFormatException. This will ensure that your application can handle malformed or inconsistent data gracefully.
Integer Overflow and Negative Stock: Make sure to handle potential integer overflow in the increase operation and prevent negative stock values in the decrease operation. These checks are crucial for maintaining logical consistency in your inventory system.

Once these issues are addressed, your code will be much stronger and ready for approval. Keep up the great work, and don't hesitate to reach out if you have any questions or need further guidance! 🚀

✨ 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-11-05T18:34:55Z

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

+    @Override
+    public void decrease(String product, Integer amount) {
+        if (contain(product)) {
+            Storage.stock.put(product, get(product) - amount);


Consider adding a check to ensure that the resulting stock amount is not negative after decreasing. This will prevent logical errors where stock becomes negative, which is usually not desired in inventory systems.

mateacademy-ai-mentor · 2024-11-05T18:34:55Z

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

+    @Override
+    public void increase(String product, Integer amount) {
+        if (contain(product)) {
+            Storage.stock.put(product, get(product) + amount);


Ensure that the increase operation does not lead to integer overflow. Although unlikely with typical inventory amounts, it's a good practice to handle such cases.

mateacademy-ai-mentor · 2024-11-05T18:34:55Z

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

+        OperationStrategy operationStrategy = new OperationStrategyImpl(operationMap);
+
+        try {
+            BufferedReader reader = new BufferedReader(new FileReader(FILE_NAME));


Consider using a try-with-resources statement for BufferedReader to ensure that the resource is closed automatically, even if an exception occurs. This will help prevent resource leaks.

mateacademy-ai-mentor · 2024-11-05T18:34:55Z

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

+                line = reader.readLine();
+            }
+        } catch (IOException e) {
+            throw new RuntimeException(e);


While rethrowing the IOException as a RuntimeException is one way to handle it, consider providing a more descriptive error message to aid in debugging. This can help identify the source of the error more easily.

mateacademy-ai-mentor · 2024-11-05T18:34:55Z

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

+                String[] splitDataFromCurrentLine = line.split(",");
+                operationStrategy.getOperation(splitDataFromCurrentLine[0])
+                        .update(splitDataFromCurrentLine[1],
+                                Integer.parseInt(splitDataFromCurrentLine[2]));


Ensure that the CSV file format is consistent and that there are no missing or malformed entries. Consider adding validation checks for splitDataFromCurrentLine to handle potential ArrayIndexOutOfBoundsException or NumberFormatException when parsing the data.

mateacademy-ai-mentor · 2024-11-05T18:34:55Z

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

+
+    @Override
+    public Operation getOperation(String operationSymbol) {
+        return operationMap.get(operationSymbol);


Consider adding a check for null return values when the operation symbol is not found in the map. You could throw an exception or return a default operation to handle such cases gracefully.

mateacademy-ai-mentor · 2024-11-05T18:34:56Z

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

+    @Override
+    public void prepare() {
+        try {
+            BufferedWriter writer = new BufferedWriter(new FileWriter(FILE_NAME));


Consider using a try-with-resources statement for BufferedWriter to ensure that the resource is closed automatically, even if an exception occurs. This will help prevent resource leaks.

mateacademy-ai-mentor · 2024-11-05T18:34:56Z

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

+            writer.close();
+
+        } catch (IOException e) {
+            throw new RuntimeException(e);


While rethrowing the IOException as a RuntimeException is one way to handle it, consider providing a more descriptive error message to aid in debugging. This can help identify the source of the error more easily.

…unknown operation symbol defense added 4. Integer Overflow and Negative Stock defence added 5. data validation added

mateacademy-ai-mentor

Great job on implementing the solution! 🎉 While the code is functional, there are several areas for improvement to ensure robustness and adherence to best practices:

File Management: Avoid hardcoding file names directly in the classes. Instead, consider passing them as parameters or using a configuration file. This will enhance flexibility and maintainability.
Exception Handling: Improve exception messages by including more context, such as file names or problematic values. This will aid in debugging and understanding errors.
Logical Errors: Address the logical issues in the increase and decrease methods of the StockDaoStorageImpl class to prevent incorrect stock management.
Data Consistency: Using a static HashMap for stock data can lead to consistency issues, especially in multi-threaded environments. Consider using dependency injection for better management.
Service Independence: Ensure services are independent and called in the correct order, adhering to SOLID principles. This will improve the modularity and scalability of your code.

These improvements are crucial for enhancing the quality and reliability of your code. Keep up the good 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-11-05T19:26:44Z

src/main/java/core/basesyntax/Mail.java

+    public static void main(String[] args) {
+        Storage storage = new Storage();
+
+        Inventory inventory = new InventoryFromCsv();


Avoid hardcoding file names outside of the Main class. Consider passing the file name as a parameter to the InventoryFromCsv class or using a configuration file to manage such constants .

mateacademy-ai-mentor · 2024-11-05T19:26:44Z

src/main/java/core/basesyntax/Mail.java

+        Inventory inventory = new InventoryFromCsv();
+        inventory.synchronizeWithTheStorage();
+
+        Report report = new ReportToCsv();


Ensure that services are independent and called in the correct order. The checklist suggests that each service should return specific data types and pass them to the next service, maintaining independence and adhering to SOLID principles .

mateacademy-ai-mentor · 2024-11-05T19:26:45Z

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

+        if (contain(product)) {
+            Storage.stock.put(product, get(product) - amount);
+        } else if (get(product) - amount < 0) {
+            throw new RuntimeException(
+                    "the quantity being subtracted is greater than the stock value");
+        } else {
+            throw new RuntimeException("There was no such product before");
+        }


The logic in the decrease method is flawed. The check for get(product) - amount < 0 should be inside the if (contain(product)) block. Currently, it is incorrectly placed in the else if block, which will never be executed because the else block will be executed if the product does not exist.

mateacademy-ai-mentor · 2024-11-05T19:26:45Z

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

+        } else if (get(product) + amount < 0) {
+            throw new RuntimeException("amount out limit");


The condition get(product) + amount < 0 in the increase method is unnecessary and logically incorrect. Since amount is being added, it should not be negative. Consider removing this condition.

mateacademy-ai-mentor · 2024-11-05T19:26:45Z

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

+import java.util.Map;
+
+public class InventoryFromCsv implements Inventory {
+    private static final String FILE_NAME = "inventory.csv";


Avoid hardcoding the file name directly in the class. Consider passing the file name as a parameter to the constructor or using a configuration file to manage such constants, as suggested in the checklist.

mateacademy-ai-mentor · 2024-11-05T19:26:45Z

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

+                line = reader.readLine();
+            }
+        } catch (IOException e) {
+            throw new RuntimeException("Cant read from file", e);


The exception message 'Cant read from file' should be more descriptive. Consider including the file name in the message to provide more context.

mateacademy-ai-mentor · 2024-11-05T19:26:45Z

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

+        if (operationMap.containsKey(operationSymbol)) {
+            return operationMap.get(operationSymbol);
+        } else {
+            throw new RuntimeException("unknown operation symbol");


The exception message 'unknown operation symbol' could be more descriptive by including the actual symbol that caused the error. This would help in debugging and understanding the issue.

mateacademy-ai-mentor · 2024-11-05T19:26:45Z

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

+import java.io.IOException;
+
+public class ReportToCsv implements Report {
+    private static final String FILE_NAME = "report.csv";


Avoid hardcoding the file name directly in the class. Consider passing the file name as a parameter to the constructor or using a configuration file to manage such constants, as suggested in the checklist.

mateacademy-ai-mentor · 2024-11-05T19:26:46Z

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

+                writer.write(product + "," + stockDao.get(product) + System.lineSeparator());
+            }
+        } catch (IOException e) {
+            throw new RuntimeException("Cant print to file", e);


The exception message 'Cant print to file' should be more descriptive. Consider including the file name in the message to provide more context.

mateacademy-ai-mentor · 2024-11-05T19:26:46Z

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

+import java.util.HashMap;
+
+public class Storage {
+    public static final HashMap<String, Integer> stock = new HashMap<>();


Using a static HashMap for storing stock data can lead to potential issues with data consistency and thread safety. Consider using a more robust solution, such as dependency injection, to manage the storage instance, especially if the application is expected to run in a multi-threaded environment.

first version

84afafe

mateacademy-ai-mentor suggested changes Nov 5, 2024

View reviewed changes

1. exception message added 2. try changed into try with resources 3. …

b14d942

…unknown operation symbol defense added 4. Integer Overflow and Negative Stock defence added 5. data validation added

MateuszSeler requested a review from mateacademy-ai-mentor November 5, 2024 19:24

mateacademy-ai-mentor approved these changes Nov 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

first version #1235

first version #1235

MateuszSeler commented Nov 5, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

mateacademy-ai-mentor Nov 5, 2024

		} else if (get(product) + amount < 0) {
		throw new RuntimeException("amount out limit");

first version #1235

Are you sure you want to change the base?

first version #1235

Conversation

MateuszSeler commented Nov 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

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

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