-
Notifications
You must be signed in to change notification settings - Fork 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
Tests for fruit shop impl #1058
base: main
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.
Good job! Let’s improve your solution;)
src/main/java/core/basesyntax/service/impl/CsvFormatReportGenerator.java
Outdated
Show resolved
Hide resolved
src/test/java/core/basesyntax/service/impl/FileWriterServiceImplTest.java
Outdated
Show resolved
Hide resolved
src/test/java/core/basesyntax/service/impl/FileWriterServiceImplTest.java
Outdated
Show resolved
Hide resolved
src/test/java/core/basesyntax/service/impl/ShopServiceImplTest.java
Outdated
Show resolved
Hide resolved
src/test/java/core/basesyntax/service/impl/operation/PurchaseOperationTest.java
Outdated
Show resolved
Hide resolved
src/test/java/core/basesyntax/strategy/OperationStrategyImplTest.java
Outdated
Show resolved
Hide resolved
private final FileReaderService fileReader; | ||
private final DataConverterService dataConverter; | ||
private final OperationStrategy operationStrategy; | ||
private final ShopService shopService; | ||
private final ReportGenerator reportGenerator; | ||
private final FileWriterService fileWriter; | ||
|
||
public Main(FileReaderService fileReader, | ||
DataConverterService dataConverter, | ||
OperationStrategy operationStrategy, | ||
ShopService shopService, | ||
ReportGenerator reportGenerator, | ||
FileWriterService fileWriter) { | ||
this.fileReader = fileReader; | ||
this.dataConverter = dataConverter; | ||
this.operationStrategy = operationStrategy; | ||
this.shopService = shopService; | ||
this.reportGenerator = reportGenerator; | ||
this.fileWriter = fileWriter; | ||
} |
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.
Let's remove it. We don't need to create an instance of Main class, because we have static method main().
In general, it's the right way to declare dependencies and initialize it. You will do the same in book-store project, for example
private final FileReaderService fileReader; | |
private final DataConverterService dataConverter; | |
private final OperationStrategy operationStrategy; | |
private final ShopService shopService; | |
private final ReportGenerator reportGenerator; | |
private final FileWriterService fileWriter; | |
public Main(FileReaderService fileReader, | |
DataConverterService dataConverter, | |
OperationStrategy operationStrategy, | |
ShopService shopService, | |
ReportGenerator reportGenerator, | |
FileWriterService fileWriter) { | |
this.fileReader = fileReader; | |
this.dataConverter = dataConverter; | |
this.operationStrategy = operationStrategy; | |
this.shopService = shopService; | |
this.reportGenerator = reportGenerator; | |
this.fileWriter = fileWriter; | |
} | |
private final FileReaderService fileReader; | |
private final DataConverterService dataConverter; | |
private final OperationStrategy operationStrategy; | |
private final ShopService shopService; | |
private final ReportGenerator reportGenerator; | |
private final FileWriterService fileWriter; | |
public Main(FileReaderService fileReader, | |
DataConverterService dataConverter, | |
OperationStrategy operationStrategy, | |
ShopService shopService, | |
ReportGenerator reportGenerator, | |
FileWriterService fileWriter) { | |
this.fileReader = fileReader; | |
this.dataConverter = dataConverter; | |
this.operationStrategy = operationStrategy; | |
this.shopService = shopService; | |
this.reportGenerator = reportGenerator; | |
this.fileWriter = fileWriter; | |
} |
public void run(String inputFilePath, String outputFilePath) { | ||
// 1. Read the data from the input CSV file | ||
List<String> inputReport = fileReader.read(inputFilePath); | ||
|
||
// 2. Convert the incoming data into FruitTransactions list | ||
List<FruitTransaction> transactions = dataConverter.convertToTransaction(inputReport); | ||
|
||
// 3. Process the incoming transactions with applicable OperationHandler implementations | ||
shopService.process(transactions); | ||
|
||
// 4. Generate report based on the current Storage state | ||
String resultingReport = reportGenerator.create(); | ||
|
||
// 5. Write the received report into the destination file | ||
fileWriter.writeToFile(outputFilePath, resultingReport); | ||
} |
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.
Let's just make it static
and call in main()
method
} | ||
|
||
@Test | ||
void testWriteToFile_shouldThrowRuntimeException_whenIoExceptionOccurs() throws IOException { |
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.
You don't need this test, because you have already verified in your previous tests, that RuntimeException is thrown when an error occurs
|
||
@Override | ||
public void doOperation(String fruitName, Integer quantity) { | ||
storageDao.setQuantity(fruitName, quantity); |
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.
What happens if the number is negative?)
Let's change it and add tests for it. See if other handlers need it too
No description provided.