implemented the requirement of the task #816

RostyslavKuzmych · 2023-10-20T12:32:16Z

No description provided.

kshuryhin

Good job, one more tip - try to add <_state_> part to your test names. It will make tests more readable generally

kshuryhin · 2023-10-23T07:45:06Z

src/test/java/core/basesyntax/service/amount/BalanceActivityHandlerTest.java

+    private static final String BANANA = "banana";
+    private static final String APPLE = "apple";
+    private BalanceActivityHandler balanceActivityHandler;
+    private FruitTransactionDao fruitTransactionDao;


Do we need this variable on the class level?

kshuryhin · 2023-10-23T07:46:32Z

src/test/java/core/basesyntax/service/FruitServiceTest.java

+    private ReaderService readerService;
+    private FruitService fruitService;
+    private TypeActivityStrategy typeActivityStrategy;


Some of these variables are also not needed on the class level I guess

dima-semeniuk

Great job!

dima-semeniuk · 2023-10-24T17:25:59Z

src/test/java/core/basesyntax/service/amount/ReturnActivityHandlerTest.java

+    @BeforeEach
+    void beforeEach() {
+
+    }


I guess this is redundant

dima-semeniuk · 2023-10-24T17:29:34Z

src/test/java/core/basesyntax/service/amount/SupplyActivityHandlerTest.java

+    @BeforeEach
+    void beforeEach() {
+
+    }


It is redundant too

bhdnchui

Good job, but a few comments need to be processed.

bhdnchui · 2023-10-25T08:07:10Z

src/test/java/core/basesyntax/dao/FruitTransactionDaoImplTest.java

+    }
+
+    @Test
+    void get_FromStorage_isOk() {


First letter after _ should be lower case.

Here you test get and store in the same test.
Separate to two tests:

store with Storage and get with fruitTransactionDao

store with fruitTransactionDao and get with Storage

bhdnchui · 2023-10-25T08:10:17Z

src/test/java/core/basesyntax/dao/FruitTransactionDaoImplTest.java

+        assertThrows(RuntimeException.class,
+                () -> fruitTransactionDao.addToStorage(null));


I think it is a bad opportunity to get RuntimeException

bhdnchui · 2023-10-25T08:15:23Z

src/test/java/core/basesyntax/model/FruitTransactionTest.java

+    void subtract_IncorrectQuantity_isNotOk() {
+        FruitTransaction fruitTransaction
+                = FruitTransaction.of(Operation.RETURN, BANANA, 20);
+        assertThrows(RuntimeException.class,


Let's not throw RuntimeException.
Also you can add check for a message.

bhdnchui · 2023-10-25T08:16:00Z

src/test/java/core/basesyntax/service/FruitServiceTest.java

+        activityHandlerMap = new HashMap<>();
+
+        activityHandlerMap.put(Operation.BALANCE,
+                new BalanceActivityHandler(new FruitTransactionDaoImpl()));


You can make new FruitTransactionDaoImpl() one time.

bhdnchui · 2023-10-25T08:16:46Z

src/test/java/core/basesyntax/service/FruitServiceTest.java

+    }
+
+    @Test
+    void write_Report_isOk() {


Find a better name.

bhdnchui · 2023-10-25T08:22:29Z

src/test/java/core/basesyntax/service/amount/BalanceActivityHandlerTest.java

+    @BeforeEach
+    void beforeEach() {
+
+    }


Suggested change

@BeforeEach

void beforeEach() {

}

bhdnchui · 2023-10-25T08:22:56Z

src/test/java/core/basesyntax/service/amount/PurchaseActivityHandlerTest.java

+    private static final String APPLE = "apple";
+
+    @BeforeEach
+    void beforeEach() {


bhdnchui · 2023-10-25T08:24:53Z

src/test/java/core/basesyntax/service/amount/PurchaseActivityHandlerTest.java

+        purchaseActivityHandler
+                .setAmountOfFruit(FruitTransaction.of(Operation.PURCHASE, BANANA, 10));
+
+        purchaseActivityHandler
+                .setAmountOfFruit(FruitTransaction.of(Operation.PURCHASE, APPLE, 40));


Let's make a two separate tests here.

It is actual for all ActivityHandlersImpl

bhdnchui · 2023-10-25T08:26:53Z

src/test/java/core/basesyntax/strategy/TypeActivityStrategyTest.java

+
+    @Test
+    void get_HandlerFromNull_isNotOk() {
+        assertThrows(NullPointerException.class,


It is not ok. You should handle such case in code.

bhdnchui · 2023-10-25T08:29:24Z

src/test/java/core/basesyntax/service/WriterServiceTest.java

+
+class WriterServiceTest {
+    private static final String EXIST_FILE = "newFile.CSV";
+    private static final String NON_EXIST_FILE = "file1.CSV";


You add such file to you PR.

d1sam

Well done, but please better choose variable names.

d1sam · 2023-10-27T08:57:01Z

src/test/java/core/basesyntax/service/ParserServiceTest.java

+        FruitTransaction fruitTransaction
+                = FruitTransaction.of(Operation.BALANCE, APPLE, 100);
+
+        FruitTransaction fruitTransaction1
+                = FruitTransaction.of(Operation.RETURN, APPLE, 50);
+
+        FruitTransaction fruitTransaction2
+                = FruitTransaction.of(Operation.BALANCE, BANANA, 100);
+
+        FruitTransaction fruitTransaction3
+                = FruitTransaction.of(Operation.PURCHASE, BANANA, 20);
+
+        FruitTransaction fruitTransaction4
+                = FruitTransaction.of(Operation.SUPPLY, APPLE, 50);


Please choose better names for variables. Without ending 1,2,3..

RostyslavKuzmych added 5 commits October 19, 2023 11:25

test version

ff1d7b4

arised unexpected exception

14ea8f6

test version

217982b

implemented the code

99643d5

fixed the mistake with mvn clean package

07611a0

kshuryhin suggested changes Oct 23, 2023

View reviewed changes

improved the readability of the code

0acf792

RostyslavKuzmych requested a review from kshuryhin October 23, 2023 09:00

improved the methods' names

54766b5

dima-semeniuk approved these changes Oct 24, 2023

View reviewed changes

bhdnchui suggested changes Oct 25, 2023

View reviewed changes

fixed a few mistakes

e9f0e98

d1sam approved these changes Oct 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implemented the requirement of the task #816

implemented the requirement of the task #816

RostyslavKuzmych commented Oct 20, 2023

kshuryhin left a comment

kshuryhin Oct 23, 2023

kshuryhin Oct 23, 2023

dima-semeniuk left a comment

dima-semeniuk Oct 24, 2023

dima-semeniuk Oct 24, 2023

bhdnchui left a comment

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

bhdnchui Oct 25, 2023

d1sam left a comment

d1sam Oct 27, 2023

		assertThrows(RuntimeException.class,
		() -> fruitTransactionDao.addToStorage(null));

implemented the requirement of the task #816

Are you sure you want to change the base?

implemented the requirement of the task #816

Conversation

RostyslavKuzmych commented Oct 20, 2023

kshuryhin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dima-semeniuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhdnchui 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d1sam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment