Skip to content

Commit ab2c12e

Browse files
committed
1) Refactored LotteryNumbers to use Joiner from guava library to join lottery numbers. 2) Solved potential thread safety issue in LotteryTicketId class, where it was using raw primitive value and incrementing it which is not thread-safe. So used AtomicInteger for brevity 3) assertEquals arguments were in incorrect order at many places, so changed order of those 4) Replaced assertFalse and assertTrue at some places with assertEquals and assertNotEquals for reducing complexity of code 5) Removed public modifiers from test cases, as they are no more needed by JUnit 5
1 parent db33cc5 commit ab2c12e

15 files changed

+76
-93
lines changed

hexagonal/src/main/java/com/iluwatar/hexagonal/database/MongoTicketRepository.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -110,21 +110,6 @@ public int getNextId() {
110110
return result.getInteger("seq");
111111
}
112112

113-
/**
114-
* @return mongo client
115-
*/
116-
public MongoClient getMongoClient() {
117-
return mongoClient;
118-
}
119-
120-
/**
121-
*
122-
* @return mongo database
123-
*/
124-
public MongoDatabase getMongoDatabase() {
125-
return database;
126-
}
127-
128113
/**
129114
*
130115
* @return tickets collection

hexagonal/src/main/java/com/iluwatar/hexagonal/domain/LotteryNumbers.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
*/
2323
package com.iluwatar.hexagonal.domain;
2424

25+
import com.google.common.base.Joiner;
26+
2527
import java.util.Collections;
2628
import java.util.HashSet;
2729
import java.util.PrimitiveIterator;
@@ -84,15 +86,7 @@ public Set<Integer> getNumbers() {
8486
* @return numbers as comma separated string
8587
*/
8688
public String getNumbersAsString() {
87-
StringBuilder builder = new StringBuilder();
88-
Iterator<Integer> iterator = numbers.iterator();
89-
for (int i = 0; i < NUM_NUMBERS; i++) {
90-
builder.append(iterator.next());
91-
if (i < NUM_NUMBERS - 1) {
92-
builder.append(",");
93-
}
94-
}
95-
return builder.toString();
89+
return Joiner.on(',').join(numbers);
9690
}
9791

9892
/**

hexagonal/src/main/java/com/iluwatar/hexagonal/domain/LotteryTicketId.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,19 @@
2222
*/
2323
package com.iluwatar.hexagonal.domain;
2424

25+
import java.util.concurrent.atomic.AtomicInteger;
26+
import java.util.concurrent.atomic.LongAdder;
27+
2528
/**
2629
* Lottery ticked id
2730
*/
2831
public class LotteryTicketId {
2932

30-
private static volatile int numAllocated;
33+
private static AtomicInteger numAllocated = new AtomicInteger(0);
3134
private final int id;
3235

3336
public LotteryTicketId() {
34-
this.id = numAllocated + 1;
35-
numAllocated++;
37+
this.id = numAllocated.incrementAndGet();
3638
}
3739

3840
public LotteryTicketId(int id) {

hexagonal/src/test/java/com/iluwatar/hexagonal/AppTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@
2727
/**
2828
* Unit test for simple App.
2929
*/
30-
public class AppTest {
30+
class AppTest {
3131

3232
@Test
33-
public void testApp() {
33+
void testApp() {
3434
String[] args = {};
3535
App.main(args);
3636
}

hexagonal/src/test/java/com/iluwatar/hexagonal/banking/InMemoryBankTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,19 @@
3232
* Tests for banking
3333
*
3434
*/
35-
public class InMemoryBankTest {
35+
class InMemoryBankTest {
3636

3737
private final WireTransfers bank = new InMemoryBank();
3838

3939
@Test
40-
public void testInit() {
41-
assertEquals(bank.getFunds("foo"), 0);
40+
void testInit() {
41+
assertEquals(0, bank.getFunds("foo"));
4242
bank.setFunds("foo", 100);
43-
assertEquals(bank.getFunds("foo"), 100);
43+
assertEquals(100, bank.getFunds("foo"));
4444
bank.setFunds("bar", 150);
45-
assertEquals(bank.getFunds("bar"), 150);
45+
assertEquals(150, bank.getFunds("bar"));
4646
assertTrue(bank.transferFunds(50, "bar", "foo"));
47-
assertEquals(bank.getFunds("foo"), 150);
48-
assertEquals(bank.getFunds("bar"), 100);
47+
assertEquals(150, bank.getFunds("foo"));
48+
assertEquals(100, bank.getFunds("bar"));
4949
}
5050
}

hexagonal/src/test/java/com/iluwatar/hexagonal/banking/MongoBankTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@
3434
* Tests for Mongo banking adapter
3535
*/
3636
@Disabled
37-
public class MongoBankTest {
37+
class MongoBankTest {
3838

3939
private static final String TEST_DB = "lotteryDBTest";
4040
private static final String TEST_ACCOUNTS_COLLECTION = "testAccounts";
4141

4242
private MongoBank mongoBank;
4343

4444
@BeforeEach
45-
public void init() {
45+
void init() {
4646
MongoConnectionPropertiesLoader.load();
4747
MongoClient mongoClient = new MongoClient(System.getProperty("mongo-host"),
4848
Integer.parseInt(System.getProperty("mongo-port")));
@@ -52,12 +52,12 @@ public void init() {
5252
}
5353

5454
@Test
55-
public void testSetup() {
55+
void testSetup() {
5656
assertEquals(0, mongoBank.getAccountsCollection().count());
5757
}
5858

5959
@Test
60-
public void testFundTransfers() {
60+
void testFundTransfers() {
6161
assertEquals(0, mongoBank.getFunds("000-000"));
6262
mongoBank.setFunds("000-000", 10);
6363
assertEquals(10, mongoBank.getFunds("000-000"));

hexagonal/src/test/java/com/iluwatar/hexagonal/database/InMemoryTicketRepositoryTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,23 +38,23 @@
3838
* Tests for {@link LotteryTicketRepository}
3939
*
4040
*/
41-
public class InMemoryTicketRepositoryTest {
41+
class InMemoryTicketRepositoryTest {
4242

4343
private final LotteryTicketRepository repository = new InMemoryTicketRepository();
4444

4545
@BeforeEach
46-
public void clear() {
46+
void clear() {
4747
repository.deleteAll();
4848
}
4949

5050
@Test
51-
public void testCrudOperations() {
51+
void testCrudOperations() {
5252
LotteryTicketRepository repository = new InMemoryTicketRepository();
53-
assertEquals(repository.findAll().size(), 0);
53+
assertTrue(repository.findAll().isEmpty());
5454
LotteryTicket ticket = LotteryTestUtils.createLotteryTicket();
5555
Optional<LotteryTicketId> id = repository.save(ticket);
5656
assertTrue(id.isPresent());
57-
assertEquals(repository.findAll().size(), 1);
57+
assertEquals(1, repository.findAll().size());
5858
Optional<LotteryTicket> optionalTicket = repository.findById(id.get());
5959
assertTrue(optionalTicket.isPresent());
6060
}

hexagonal/src/test/java/com/iluwatar/hexagonal/database/MongoTicketRepositoryTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
* Tests for Mongo based ticket repository
4242
*/
4343
@Disabled
44-
public class MongoTicketRepositoryTest {
44+
class MongoTicketRepositoryTest {
4545

4646
private static final String TEST_DB = "lotteryTestDB";
4747
private static final String TEST_TICKETS_COLLECTION = "lotteryTestTickets";
@@ -50,7 +50,7 @@ public class MongoTicketRepositoryTest {
5050
private MongoTicketRepository repository;
5151

5252
@BeforeEach
53-
public void init() {
53+
void init() {
5454
MongoConnectionPropertiesLoader.load();
5555
MongoClient mongoClient = new MongoClient(System.getProperty("mongo-host"),
5656
Integer.parseInt(System.getProperty("mongo-port")));
@@ -61,20 +61,20 @@ public void init() {
6161
}
6262

6363
@Test
64-
public void testSetup() {
65-
assertTrue(repository.getCountersCollection().count() == 1);
66-
assertTrue(repository.getTicketsCollection().count() == 0);
64+
void testSetup() {
65+
assertEquals(1, repository.getCountersCollection().count());
66+
assertEquals(0, repository.getTicketsCollection().count());
6767
}
6868

6969
@Test
70-
public void testNextId() {
70+
void testNextId() {
7171
assertEquals(1, repository.getNextId());
7272
assertEquals(2, repository.getNextId());
7373
assertEquals(3, repository.getNextId());
7474
}
7575

7676
@Test
77-
public void testCrudOperations() {
77+
void testCrudOperations() {
7878
// create new lottery ticket and save it
7979
PlayerDetails details = new PlayerDetails("foo@bar.com", "123-123", "07001234");
8080
LotteryNumbers random = LotteryNumbers.createRandom();

hexagonal/src/test/java/com/iluwatar/hexagonal/domain/LotteryNumbersTest.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
import java.util.HashSet;
2929

3030
import static org.junit.jupiter.api.Assertions.assertEquals;
31-
import static org.junit.jupiter.api.Assertions.assertFalse;
31+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
3232
import static org.junit.jupiter.api.Assertions.assertThrows;
3333
import static org.junit.jupiter.api.Assertions.assertTrue;
3434

@@ -37,10 +37,10 @@
3737
* Unit tests for {@link LotteryNumbers}
3838
*
3939
*/
40-
public class LotteryNumbersTest {
40+
class LotteryNumbersTest {
4141

4242
@Test
43-
public void testGivenNumbers() {
43+
void testGivenNumbers() {
4444
LotteryNumbers numbers = LotteryNumbers.create(
4545
new HashSet<>(Arrays.asList(1, 2, 3, 4)));
4646
assertEquals(numbers.getNumbers().size(), 4);
@@ -51,7 +51,7 @@ public void testGivenNumbers() {
5151
}
5252

5353
@Test
54-
public void testNumbersCantBeModified() {
54+
void testNumbersCantBeModified() {
5555
LotteryNumbers numbers = LotteryNumbers.create(
5656
new HashSet<>(Arrays.asList(1, 2, 3, 4)));
5757
assertThrows(UnsupportedOperationException.class, () -> {
@@ -60,20 +60,20 @@ public void testNumbersCantBeModified() {
6060
}
6161

6262
@Test
63-
public void testRandomNumbers() {
63+
void testRandomNumbers() {
6464
LotteryNumbers numbers = LotteryNumbers.createRandom();
6565
assertEquals(numbers.getNumbers().size(), LotteryNumbers.NUM_NUMBERS);
6666
}
6767

6868
@Test
69-
public void testEquals() {
69+
void testEquals() {
7070
LotteryNumbers numbers1 = LotteryNumbers.create(
7171
new HashSet<>(Arrays.asList(1, 2, 3, 4)));
7272
LotteryNumbers numbers2 = LotteryNumbers.create(
7373
new HashSet<>(Arrays.asList(1, 2, 3, 4)));
74-
assertTrue(numbers1.equals(numbers2));
74+
assertEquals(numbers1, numbers2);
7575
LotteryNumbers numbers3 = LotteryNumbers.create(
7676
new HashSet<>(Arrays.asList(11, 12, 13, 14)));
77-
assertFalse(numbers1.equals(numbers3));
77+
assertNotEquals(numbers1, numbers3);
7878
}
7979
}

hexagonal/src/test/java/com/iluwatar/hexagonal/domain/LotteryTest.java

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,15 @@
3838
import java.util.Optional;
3939

4040
import static org.junit.jupiter.api.Assertions.assertEquals;
41+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
4142
import static org.junit.jupiter.api.Assertions.assertTrue;
4243

4344
/**
4445
*
4546
* Test the lottery system
4647
*
4748
*/
48-
public class LotteryTest {
49+
class LotteryTest {
4950

5051
private Injector injector;
5152
@Inject
@@ -55,22 +56,22 @@ public class LotteryTest {
5556
@Inject
5657
private WireTransfers wireTransfers;
5758

58-
public LotteryTest() {
59+
LotteryTest() {
5960
this.injector = Guice.createInjector(new LotteryTestingModule());
6061
}
6162

6263
@BeforeEach
63-
public void setup() {
64+
void setup() {
6465
injector.injectMembers(this);
6566
// add funds to the test player's bank account
6667
wireTransfers.setFunds("123-12312", 100);
6768
}
6869

6970
@Test
70-
public void testLottery() {
71+
void testLottery() {
7172
// admin resets the lottery
7273
administration.resetLottery();
73-
assertEquals(administration.getAllSubmittedTickets().size(), 0);
74+
assertEquals(0, administration.getAllSubmittedTickets().size());
7475

7576
// players submit the lottery tickets
7677
Optional<LotteryTicketId> ticket1 = service.submitTicket(LotteryTestUtils.createLotteryTicket("cvt@bbb.com",
@@ -82,7 +83,7 @@ public void testLottery() {
8283
Optional<LotteryTicketId> ticket3 = service.submitTicket(LotteryTestUtils.createLotteryTicket("arg@boo.com",
8384
"123-12312", "+32421255", new HashSet<>(Arrays.asList(6, 8, 13, 19))));
8485
assertTrue(ticket3.isPresent());
85-
assertEquals(administration.getAllSubmittedTickets().size(), 3);
86+
assertEquals(3, administration.getAllSubmittedTickets().size());
8687

8788
// perform lottery
8889
LotteryNumbers winningNumbers = administration.performLottery();
@@ -91,23 +92,23 @@ public void testLottery() {
9192
Optional<LotteryTicketId> ticket4 = service.submitTicket(LotteryTestUtils.createLotteryTicket("lucky@orb.com",
9293
"123-12312", "+12421255", winningNumbers.getNumbers()));
9394
assertTrue(ticket4.isPresent());
94-
assertEquals(administration.getAllSubmittedTickets().size(), 4);
95+
assertEquals(4, administration.getAllSubmittedTickets().size());
9596

9697
// check winners
9798
Map<LotteryTicketId, LotteryTicket> tickets = administration.getAllSubmittedTickets();
9899
for (LotteryTicketId id: tickets.keySet()) {
99100
LotteryTicketCheckResult checkResult = service.checkTicketForPrize(id, winningNumbers);
100-
assertTrue(checkResult.getResult() != CheckResult.TICKET_NOT_SUBMITTED);
101+
assertNotEquals(CheckResult.TICKET_NOT_SUBMITTED, checkResult.getResult());
101102
if (checkResult.getResult().equals(CheckResult.WIN_PRIZE)) {
102103
assertTrue(checkResult.getPrizeAmount() > 0);
103104
} else if (checkResult.getResult().equals(CheckResult.WIN_PRIZE)) {
104-
assertEquals(checkResult.getPrizeAmount(), 0);
105+
assertEquals(0, checkResult.getPrizeAmount());
105106
}
106107
}
107108

108109
// check another ticket that has not been submitted
109110
LotteryTicketCheckResult checkResult = service.checkTicketForPrize(new LotteryTicketId(), winningNumbers);
110-
assertTrue(checkResult.getResult() == CheckResult.TICKET_NOT_SUBMITTED);
111-
assertEquals(checkResult.getPrizeAmount(), 0);
111+
assertEquals(CheckResult.TICKET_NOT_SUBMITTED, checkResult.getResult());
112+
assertEquals(0, checkResult.getPrizeAmount());
112113
}
113114
}

hexagonal/src/test/java/com/iluwatar/hexagonal/domain/LotteryTicketCheckResultTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,21 @@
2626
import org.junit.jupiter.api.Test;
2727

2828
import static org.junit.jupiter.api.Assertions.assertEquals;
29-
import static org.junit.jupiter.api.Assertions.assertFalse;
29+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
3030

3131
/**
3232
*
3333
* Unit tests for {@link LotteryTicketCheckResult}
3434
*
3535
*/
36-
public class LotteryTicketCheckResultTest {
36+
class LotteryTicketCheckResultTest {
3737

3838
@Test
39-
public void testEquals() {
39+
void testEquals() {
4040
LotteryTicketCheckResult result1 = new LotteryTicketCheckResult(CheckResult.NO_PRIZE);
4141
LotteryTicketCheckResult result2 = new LotteryTicketCheckResult(CheckResult.NO_PRIZE);
4242
assertEquals(result1, result2);
4343
LotteryTicketCheckResult result3 = new LotteryTicketCheckResult(CheckResult.WIN_PRIZE, 300000);
44-
assertFalse(result1.equals(result3));
44+
assertNotEquals(result1, result3);
4545
}
4646
}

0 commit comments

Comments
 (0)