-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
create fruit shop #1229
base: master
Are you sure you want to change the base?
create fruit shop #1229
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your hard work on this project! While your code meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:
-
Error Handling: Throwing
RuntimeException
for various operations (unsupported operations, insufficient stock,IOException
, etc.) can lead to abrupt application termination. Consider using custom exceptions or handling errors more gracefully to provide better feedback to users. -
Null Checks: Ensure that methods like
operationStrategy.get(transaction.getOperation())
andgetOperationFromCode
handle potential null values or invalid inputs to preventNullPointerException
or application crashes. -
Input Validation: Add validation checks for input data, such as ensuring the correct number of elements in arrays and handling
NumberFormatException
for parsing integers. This will help avoid runtime exceptions due to malformed input. -
Thread Safety: If your service is intended for a multi-threaded environment, consider using thread-safe collections like
ConcurrentHashMap
. -
File Handling: Ensure file paths are correct and files exist before reading to avoid runtime errors. Add error handling for file operations.
-
Clarify Comments: The comment suggesting the removal of the main class could be confusing. Clarify its purpose or remove it if not applicable.
Addressing these issues will significantly improve the robustness and user experience of your application. 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.
import java.util.Map; | ||
|
||
/** | ||
* Feel free to remove this class and create your own. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment suggests removing this class, which might be confusing if this is intended to be the main entry point of the application. Consider clarifying the purpose of this comment or removing it if it's not applicable.
List<String> inputReport = fileReader.read( | ||
"src/main/java/core/basesyntax/reportToRead.csv"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the file path is correct and that the file exists to avoid runtime errors. Consider adding error handling for file reading operations.
|
||
// 4. Process the incoming transactions with applicable OperationHandler implementations | ||
DataConverter dataConverter = new DataConverterImpl(); | ||
List<FruitTransaction> transactions = dataConverter.convertToTransaction(inputReport); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the convertToTransaction
method correctly handles any potential formatting issues in the input data to avoid runtime exceptions.
if (line.startsWith("type")) { | ||
continue; | ||
} | ||
String[] parts = line.split(","); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding validation to check if the parts
array has the expected number of elements before accessing them. This will prevent ArrayIndexOutOfBoundsException
if the input data is malformed.
String[] parts = line.split(","); | ||
String operationCode = parts[0]; | ||
String fruit = parts[1]; | ||
int quantity = Integer.parseInt(parts[2]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for Integer.parseInt(parts[2])
to catch NumberFormatException
in case the quantity is not a valid integer.
|
||
@Override | ||
public OperationHandler get(FruitTransaction.Operation operation) { | ||
return handlerMap.get(operation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The get
method may return null if the operation is not found in the handlerMap
. Consider adding a check to handle this case, possibly by throwing an exception or returning a default handler, to avoid potential NullPointerException
in the calling code.
int currentQuantity = shopService.getQuantity(fruit); | ||
if (transaction.getOperation() == FruitTransaction.Operation.PURCHASE) { | ||
if (transactionQuantity > currentQuantity) { | ||
throw new RuntimeException("Not enough " + fruit + " in stock for purchase"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing a RuntimeException
for insufficient stock might not be the best approach as it can terminate the application abruptly. Consider using a custom exception or handling the error more gracefully to provide better feedback to the user.
} | ||
shopService.addFruits(fruit, currentQuantity - transactionQuantity); | ||
} else { | ||
throw new RuntimeException("Unsupported operation for PurchaseHandler"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing a RuntimeException
for unsupported operations might not be the best approach. Consider using a custom exception or handling the error more gracefully to provide better feedback to the user.
int quantity = transaction.getQuantity(); | ||
shopService.addFruits(fruit, quantity); | ||
} else { | ||
throw new RuntimeException("Unsupported operation for ReturnHandler"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing a RuntimeException
for unsupported operations might not be the best approach as it can terminate the application abruptly. Consider using a custom exception or handling the error more gracefully to provide better feedback to the user.
int quantity = transaction.getQuantity(); | ||
shopService.addFruits(fruit, quantity); | ||
} else { | ||
throw new RuntimeException("Unsupported operation for SupplyHandler"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing a RuntimeException
for unsupported operations might not be the best approach as it can terminate the application abruptly. Consider using a custom exception or handling the error more gracefully to provide better feedback to the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on your submission! 🎉 While there are a few areas that need improvement, your code is on the right track. Here are some key points to consider:
-
File Paths: Ensure that the file paths are correct and that you have the necessary permissions to read from and write to these locations. Verify the paths for both
reportToRead.csv
andfinalReport.csv
. -
Data Conversion: It appears that the step to convert incoming data into
FruitTransaction
objects is missing. Make sure to implement this using theDataConverter
instance. -
Error Handling: Consider adding error handling for parsing integers to manage cases where the quantity is not a valid integer. This will prevent crashes due to malformed input data.
-
Exception Messages: When throwing exceptions, provide more context in the error message to aid in debugging, such as the line number or content.
-
SOLID Principles: Review your code to ensure it adheres to SOLID principles, particularly the Dependency Inversion Principle, to improve maintainability.
These improvements will enhance the functionality and robustness of your code. Keep up the good work, and continue to learn and grow from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
List<String> inputReport = fileReader.read( | ||
"src/main/java/core/basesyntax/reportToRead.csv"); | ||
|
||
// 2. Convert the incoming data into FruitTransactions list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like the step to convert the incoming data into a list of FruitTransaction
objects is missing. Ensure that you implement this step using the DataConverter
instance.
List<String> inputReport = fileReader.read( | ||
"src/main/java/core/basesyntax/reportToRead.csv"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify that the file path 'src/main/java/core/basesyntax/reportToRead.csv' is correct and that the file exists at this location. Otherwise, the FileReader
may throw an error.
|
||
// 6. Write the received report into the destination file | ||
FileWriter fileWriter = new FileWriterImpl(); | ||
fileWriter.write(resultingReport, "src/main/java/core/basesyntax/finalReport.csv"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify that the file path 'src/main/java/core/basesyntax/finalReport.csv' is correct and that you have write permissions to this location. Otherwise, the FileWriter
may throw an error.
String[] parts = line.split(","); | ||
String operationCode = parts[0]; | ||
String fruit = parts[1]; | ||
int quantity = Integer.parseInt(parts[2]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding error handling for Integer.parseInt(parts[2])
to manage cases where the quantity is not a valid integer. This will prevent the application from crashing due to malformed input data.
return operation; | ||
} | ||
} | ||
throw new IllegalArgumentException("Invalid operation code: " + code); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing an IllegalArgumentException
is appropriate here, but consider providing more context in the error message, such as the line number or content, to aid in debugging.
No description provided.