Skip to content

Commit

Permalink
1) Refactored LotteryNumbers to use Joiner from guava library to join…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
npathai committed Oct 15, 2018
1 parent db33cc5 commit ab2c12e
Show file tree
Hide file tree
Showing 15 changed files with 76 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -110,21 +110,6 @@ public int getNextId() {
return result.getInteger("seq");
}

/**
* @return mongo client
*/
public MongoClient getMongoClient() {
return mongoClient;
}

/**
*
* @return mongo database
*/
public MongoDatabase getMongoDatabase() {
return database;
}

/**
*
* @return tickets collection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
*/
package com.iluwatar.hexagonal.domain;

import com.google.common.base.Joiner;

import java.util.Collections;
import java.util.HashSet;
import java.util.PrimitiveIterator;
Expand Down Expand Up @@ -84,15 +86,7 @@ public Set<Integer> getNumbers() {
* @return numbers as comma separated string
*/
public String getNumbersAsString() {
StringBuilder builder = new StringBuilder();
Iterator<Integer> iterator = numbers.iterator();
for (int i = 0; i < NUM_NUMBERS; i++) {
builder.append(iterator.next());
if (i < NUM_NUMBERS - 1) {
builder.append(",");
}
}
return builder.toString();
return Joiner.on(',').join(numbers);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,19 @@
*/
package com.iluwatar.hexagonal.domain;

import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.LongAdder;

/**
* Lottery ticked id
*/
public class LotteryTicketId {

private static volatile int numAllocated;
private static AtomicInteger numAllocated = new AtomicInteger(0);
private final int id;

public LotteryTicketId() {
this.id = numAllocated + 1;
numAllocated++;
this.id = numAllocated.incrementAndGet();
}

public LotteryTicketId(int id) {
Expand Down
4 changes: 2 additions & 2 deletions hexagonal/src/test/java/com/iluwatar/hexagonal/AppTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
/**
* Unit test for simple App.
*/
public class AppTest {
class AppTest {

@Test
public void testApp() {
void testApp() {
String[] args = {};
App.main(args);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,19 @@
* Tests for banking
*
*/
public class InMemoryBankTest {
class InMemoryBankTest {

private final WireTransfers bank = new InMemoryBank();

@Test
public void testInit() {
assertEquals(bank.getFunds("foo"), 0);
void testInit() {
assertEquals(0, bank.getFunds("foo"));
bank.setFunds("foo", 100);
assertEquals(bank.getFunds("foo"), 100);
assertEquals(100, bank.getFunds("foo"));
bank.setFunds("bar", 150);
assertEquals(bank.getFunds("bar"), 150);
assertEquals(150, bank.getFunds("bar"));
assertTrue(bank.transferFunds(50, "bar", "foo"));
assertEquals(bank.getFunds("foo"), 150);
assertEquals(bank.getFunds("bar"), 100);
assertEquals(150, bank.getFunds("foo"));
assertEquals(100, bank.getFunds("bar"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@
* Tests for Mongo banking adapter
*/
@Disabled
public class MongoBankTest {
class MongoBankTest {

private static final String TEST_DB = "lotteryDBTest";
private static final String TEST_ACCOUNTS_COLLECTION = "testAccounts";

private MongoBank mongoBank;

@BeforeEach
public void init() {
void init() {
MongoConnectionPropertiesLoader.load();
MongoClient mongoClient = new MongoClient(System.getProperty("mongo-host"),
Integer.parseInt(System.getProperty("mongo-port")));
Expand All @@ -52,12 +52,12 @@ public void init() {
}

@Test
public void testSetup() {
void testSetup() {
assertEquals(0, mongoBank.getAccountsCollection().count());
}

@Test
public void testFundTransfers() {
void testFundTransfers() {
assertEquals(0, mongoBank.getFunds("000-000"));
mongoBank.setFunds("000-000", 10);
assertEquals(10, mongoBank.getFunds("000-000"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,23 @@
* Tests for {@link LotteryTicketRepository}
*
*/
public class InMemoryTicketRepositoryTest {
class InMemoryTicketRepositoryTest {

private final LotteryTicketRepository repository = new InMemoryTicketRepository();

@BeforeEach
public void clear() {
void clear() {
repository.deleteAll();
}

@Test
public void testCrudOperations() {
void testCrudOperations() {
LotteryTicketRepository repository = new InMemoryTicketRepository();
assertEquals(repository.findAll().size(), 0);
assertTrue(repository.findAll().isEmpty());
LotteryTicket ticket = LotteryTestUtils.createLotteryTicket();
Optional<LotteryTicketId> id = repository.save(ticket);
assertTrue(id.isPresent());
assertEquals(repository.findAll().size(), 1);
assertEquals(1, repository.findAll().size());
Optional<LotteryTicket> optionalTicket = repository.findById(id.get());
assertTrue(optionalTicket.isPresent());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
* Tests for Mongo based ticket repository
*/
@Disabled
public class MongoTicketRepositoryTest {
class MongoTicketRepositoryTest {

private static final String TEST_DB = "lotteryTestDB";
private static final String TEST_TICKETS_COLLECTION = "lotteryTestTickets";
Expand All @@ -50,7 +50,7 @@ public class MongoTicketRepositoryTest {
private MongoTicketRepository repository;

@BeforeEach
public void init() {
void init() {
MongoConnectionPropertiesLoader.load();
MongoClient mongoClient = new MongoClient(System.getProperty("mongo-host"),
Integer.parseInt(System.getProperty("mongo-port")));
Expand All @@ -61,20 +61,20 @@ public void init() {
}

@Test
public void testSetup() {
assertTrue(repository.getCountersCollection().count() == 1);
assertTrue(repository.getTicketsCollection().count() == 0);
void testSetup() {
assertEquals(1, repository.getCountersCollection().count());
assertEquals(0, repository.getTicketsCollection().count());
}

@Test
public void testNextId() {
void testNextId() {
assertEquals(1, repository.getNextId());
assertEquals(2, repository.getNextId());
assertEquals(3, repository.getNextId());
}

@Test
public void testCrudOperations() {
void testCrudOperations() {
// create new lottery ticket and save it
PlayerDetails details = new PlayerDetails("foo@bar.com", "123-123", "07001234");
LotteryNumbers random = LotteryNumbers.createRandom();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import java.util.HashSet;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

Expand All @@ -37,10 +37,10 @@
* Unit tests for {@link LotteryNumbers}
*
*/
public class LotteryNumbersTest {
class LotteryNumbersTest {

@Test
public void testGivenNumbers() {
void testGivenNumbers() {
LotteryNumbers numbers = LotteryNumbers.create(
new HashSet<>(Arrays.asList(1, 2, 3, 4)));
assertEquals(numbers.getNumbers().size(), 4);
Expand All @@ -51,7 +51,7 @@ public void testGivenNumbers() {
}

@Test
public void testNumbersCantBeModified() {
void testNumbersCantBeModified() {
LotteryNumbers numbers = LotteryNumbers.create(
new HashSet<>(Arrays.asList(1, 2, 3, 4)));
assertThrows(UnsupportedOperationException.class, () -> {
Expand All @@ -60,20 +60,20 @@ public void testNumbersCantBeModified() {
}

@Test
public void testRandomNumbers() {
void testRandomNumbers() {
LotteryNumbers numbers = LotteryNumbers.createRandom();
assertEquals(numbers.getNumbers().size(), LotteryNumbers.NUM_NUMBERS);
}

@Test
public void testEquals() {
void testEquals() {
LotteryNumbers numbers1 = LotteryNumbers.create(
new HashSet<>(Arrays.asList(1, 2, 3, 4)));
LotteryNumbers numbers2 = LotteryNumbers.create(
new HashSet<>(Arrays.asList(1, 2, 3, 4)));
assertTrue(numbers1.equals(numbers2));
assertEquals(numbers1, numbers2);
LotteryNumbers numbers3 = LotteryNumbers.create(
new HashSet<>(Arrays.asList(11, 12, 13, 14)));
assertFalse(numbers1.equals(numbers3));
assertNotEquals(numbers1, numbers3);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,15 @@
import java.util.Optional;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
*
* Test the lottery system
*
*/
public class LotteryTest {
class LotteryTest {

private Injector injector;
@Inject
Expand All @@ -55,22 +56,22 @@ public class LotteryTest {
@Inject
private WireTransfers wireTransfers;

public LotteryTest() {
LotteryTest() {
this.injector = Guice.createInjector(new LotteryTestingModule());
}

@BeforeEach
public void setup() {
void setup() {
injector.injectMembers(this);
// add funds to the test player's bank account
wireTransfers.setFunds("123-12312", 100);
}

@Test
public void testLottery() {
void testLottery() {
// admin resets the lottery
administration.resetLottery();
assertEquals(administration.getAllSubmittedTickets().size(), 0);
assertEquals(0, administration.getAllSubmittedTickets().size());

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

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

// check winners
Map<LotteryTicketId, LotteryTicket> tickets = administration.getAllSubmittedTickets();
for (LotteryTicketId id: tickets.keySet()) {
LotteryTicketCheckResult checkResult = service.checkTicketForPrize(id, winningNumbers);
assertTrue(checkResult.getResult() != CheckResult.TICKET_NOT_SUBMITTED);
assertNotEquals(CheckResult.TICKET_NOT_SUBMITTED, checkResult.getResult());
if (checkResult.getResult().equals(CheckResult.WIN_PRIZE)) {
assertTrue(checkResult.getPrizeAmount() > 0);
} else if (checkResult.getResult().equals(CheckResult.WIN_PRIZE)) {
assertEquals(checkResult.getPrizeAmount(), 0);
assertEquals(0, checkResult.getPrizeAmount());
}
}

// check another ticket that has not been submitted
LotteryTicketCheckResult checkResult = service.checkTicketForPrize(new LotteryTicketId(), winningNumbers);
assertTrue(checkResult.getResult() == CheckResult.TICKET_NOT_SUBMITTED);
assertEquals(checkResult.getPrizeAmount(), 0);
assertEquals(CheckResult.TICKET_NOT_SUBMITTED, checkResult.getResult());
assertEquals(0, checkResult.getPrizeAmount());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,21 @@
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;

/**
*
* Unit tests for {@link LotteryTicketCheckResult}
*
*/
public class LotteryTicketCheckResultTest {
class LotteryTicketCheckResultTest {

@Test
public void testEquals() {
void testEquals() {
LotteryTicketCheckResult result1 = new LotteryTicketCheckResult(CheckResult.NO_PRIZE);
LotteryTicketCheckResult result2 = new LotteryTicketCheckResult(CheckResult.NO_PRIZE);
assertEquals(result1, result2);
LotteryTicketCheckResult result3 = new LotteryTicketCheckResult(CheckResult.WIN_PRIZE, 300000);
assertFalse(result1.equals(result3));
assertNotEquals(result1, result3);
}
}
Loading

0 comments on commit ab2c12e

Please sign in to comment.