-
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
Implement storage management with CSV report handling. #1238
base: master
Are you sure you want to change the base?
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.
Great job on your implementation! 🎉 While there are a few minor issues related to exception message formatting and code readability, they don't impact the functionality of your code. Keep up the good work and consider addressing these minor improvements in future iterations to enhance code clarity and maintainability. Well done! 👏
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
try { | ||
return Files.readAllLines(Paths.get(filePath)); | ||
} catch (IOException e) { | ||
throw new RuntimeException("Can't read data from file: " + filePath + e); |
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 a space or separator between the file path and the exception message in the RuntimeException
to improve readability. For example, use "Can't read data from file: " + filePath + ". " + e
.
@Override | ||
public void write(String data, String filePath) { | ||
try { | ||
Files.write(Paths.get(filePath),data.getBytes()); |
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 a space after the comma in data.getBytes()
for better readability. It should be Files.write(Paths.get(filePath), data.getBytes());
.
); | ||
convertedDataFromList.add(fruitTransaction); | ||
} catch (NumberFormatException | ArrayIndexOutOfBoundsException e) { | ||
throw new RuntimeException("Can't convert data from file: " + dataToConvert + e); |
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 a space or separator between the data and the exception message in the RuntimeException
to improve readability. For example, use "Can't convert data from file: " + dataToConvert + ". " + e
.
@Override | ||
public boolean test(String element) { | ||
return !element.isEmpty() | ||
&& element.trim().toLowerCase().startsWith(INVALID_FIRST_LINE_START); |
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 logic in the test
method seems to be inverted. The method currently returns true if the line starts with INVALID_FIRST_LINE_START
, which implies it's invalid. Consider revising the logic to return false in such cases, as the method name suggests it should validate the line.
No description provided.