Basic Ticket booking code with Unit tests

Question

I am going to run a session for a few other developers who are new to Junit, mocking, etc.

I have designed a very simple application with classes and tests to demonstrate how to do unit testing. Please review the code and tests. Any suggestions are welcome.

The problem is of a sample booking service.

• BookingService talks to PaymentGateway and TicketCenter, returns different BookingResponses based on input
• PaymentGateway takes booking amount and Card, charges the card as long as the card’s available limit is more than the booking amount.
• When booking amount exceeds the Card’s available limit, Card throws a cannot-charge exception
• TicketCenter can book tickets, as long as the booked tickets count hasn’t reached the limit when TicketCenter makes a booking, adds the booking to the list it maintains

class Card

public class Card {
 private String provider;
 private double limit;
 public Card(String provider, double limit) {
 this.provider = provider;
 this.limit = limit;
 }
 public void charge(double amount) throws CannotChargeException {
 if (this.limit < amount) {
 throw new CannotChargeException("low balance");
 }
 this.limit -= amount;
 }
 public double getAvailableLimit() {
 return limit;
 }
}

class CannotChargeException

public class CannotChargeException extends Throwable {
 public CannotChargeException(String message) {
 super(message);
 }
}

class CardTest

import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.*;
class CardTest {
 @Test
 public void shouldReturnAvailableLimitAfterDeductionOfCumulativeCharges() throws CannotChargeException {
 Card card = new Card("Amex", 1000);
 card.charge(100);
 card.charge(100);
 assertEquals(800, card.getAvailableLimit());
 }
 @Test
 public void shouldThrowExceptionWhenCardChargeExceedsLimit() {
 Card card = new Card("Amex", 1000);
 assertThrows(CannotChargeException.class, () -> {
 card.charge(1100);
 }, "low balance");
 }
}

class Ticket

public class Ticket {
 private String user;
 public Ticket(String user) {
 this.user = user;
 }
 @Override
 public String toString() {
 return String.format("This ticket belongs to %s", user);
 }
 @Override
 public boolean equals(Object obj) {
 return this.user.equals(((Ticket) obj).user);
 }
}

class TickerCenter

import java.util.ArrayList;
import java.util.List;
public class TicketCenter {
 private List<Ticket> tickets;
 private int limit;
 public TicketCenter(int limit) {
 this.limit = limit;
 this.tickets = new ArrayList<>();
 }
 protected boolean canBook() {
 return this.tickets.size() < limit;
 }
 public Ticket book(String user) {
 Ticket ticket = new Ticket(user);
 tickets.add(ticket);
 return ticket;
 }
}

class TicketCenterTest

import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
class TicketCenterTest {
 @Test
 public void shouldReturnFalseWhenTicketLimitReached() {
 String johnDoe = "John Doe";
 TicketCenter ticketCenter = new TicketCenter(1);
 ticketCenter.book(johnDoe);
 assertFalse(ticketCenter.canBook());
 }
 @Test
 public void shouldReturnTrueWhenTicketLimitNotReached() {
 String johnDoe = "John Doe";
 String joeDoe = "Joe Doe";
 TicketCenter ticketCenter = new TicketCenter(3);
 ticketCenter.book(johnDoe);
 ticketCenter.book(joeDoe);
 assertTrue(ticketCenter.canBook());
 }
}

class PaymentGateway

public class PaymentGateway {
 public void charge(Card card, double amount) throws CannotChargeException {
 card.charge(amount);
 }
}

class PaymentGatewayTest

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.*;
class PaymentGatewayTest {
 private PaymentGateway paymentGateway;
 private Card card;
 @BeforeEach
 public void setup() {
 card = mock(Card.class);
 paymentGateway = new PaymentGateway();
 }
 @Test
 public void shouldInvokeCardChargeMethodWhenPaymentGatewayChargeCalled() throws CannotChargeException {
 paymentGateway.charge(card, 100.00);
 verify(card, times(1)).charge(eq(100.00));
 }
 @Test
 public void shouldThrowExceptionWhenCardThrowException() throws CannotChargeException {
 doThrow(new CannotChargeException("low charge")).when(card).charge(eq(100.00));
 assertThrows(CannotChargeException.class, () -> {
 paymentGateway.charge(card, 100.00);
 });
 }
 @Test
 public void shouldReturnBySpyCardAvailableLimitAfterPaymentGatewayCharge() throws CannotChargeException {
 Card amexCard = new Card("Amex", 1000);
 Card spyCard = Mockito.spy(amexCard);
 paymentGateway.charge(spyCard,100);
 assertEquals(900, spyCard.getAvailableLimit());
 }
}

class BookingService

import java.util.Optional;
public class BookingService {
 private PaymentGateway paymentGateway;
 private TicketCenter ticketCenter;
 public BookingService(PaymentGateway paymentGateway, TicketCenter ticketCenter) {
 this.paymentGateway = paymentGateway;
 this.ticketCenter = ticketCenter;
 }
 public BookingResponse bookTicket(String user, Card card, double amount) {
 if (ticketCenter.canBook()) {
 try {
 paymentGateway.charge(card, amount);
 } catch (CannotChargeException e) {
 return new BookingResponse(BookingStatus.PAYMENT_ERROR, Optional.empty());
 }
 return new BookingResponse(BookingStatus.SUCCESS, Optional.of(ticketCenter.book(user)));
 }
 return new BookingResponse(BookingStatus.SOLD_OUT, Optional.empty());
 }
}

enum BookingStatus

public enum BookingStatus {
 SOLD_OUT, PAYMENT_ERROR,SUCCESS;
}

class BookingResponse

import java.util.Optional;
public class BookingResponse {
 private BookingStatus bookingStatus;
 private Optional<Ticket> ticket;
 public BookingResponse(BookingStatus bookingStatus, Optional<Ticket> ticket) {
 this.bookingStatus = bookingStatus;
 this.ticket = ticket;
 }
 @Override
 public boolean equals(Object obj) {
 BookingResponse otherBookingResponse = (BookingResponse) obj;
 return otherBookingResponse.bookingStatus.equals(this.bookingStatus) && otherBookingResponse.ticket.equals(this.ticket);
 }
}

class BookingServiceTest

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import java.util.Optional;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.*;
class BookingServiceTest {
 private BookingService bookingService;
 private PaymentGateway paymentGateway;
 private TicketCenter ticketCenter;
 @BeforeEach
 public void setup() {
 paymentGateway = mock(PaymentGateway.class);
 ticketCenter = mock(TicketCenter.class);
 bookingService = new BookingService(paymentGateway, ticketCenter);
 }
 @Test
 public void shouldReturnBookingSuccessWhenTicketCenterAllowsBooking() throws CannotChargeException {
 String johnDoe = "John Doe";
 Card amex = new Card("AMEX", 1000);
 Ticket ticket = new Ticket(johnDoe);
 BookingResponse successBookingResponse = new BookingResponse(BookingStatus.SUCCESS, Optional.of(ticket));
 when(ticketCenter.canBook()).thenReturn(true);
 doNothing().when(paymentGateway).charge(eq(amex), eq(675.00));
 when(ticketCenter.book(eq(johnDoe))).thenReturn(ticket);
 BookingResponse bookingResponse = bookingService.bookTicket(johnDoe, amex, 675.00);
 assertEquals(successBookingResponse, bookingResponse);
 }
 @Test
 public void shouldReturnPaymentErrorWhenPaymentGatewayThrowsException() throws CannotChargeException {
 String johnDoe = "John Doe";
 Card visaCard = new Card("VISA", 1000);
 BookingResponse paymentErrorBookingResponse = new BookingResponse(BookingStatus.PAYMENT_ERROR, Optional.empty());
 when(ticketCenter.canBook()).thenReturn(true);
 doThrow(new CannotChargeException("low balance")).when(paymentGateway).charge(eq(visaCard), eq(500.00));
 BookingResponse actualBookingResponse = bookingService.bookTicket(johnDoe, visaCard, 500.00);
 assertEquals(paymentErrorBookingResponse, actualBookingResponse);
 }
 @Test
 public void shouldReturnSoldOutWhenTicketCenterCannotBook() throws CannotChargeException {
 String johnDoe = "John Doe";
 Card visaCard = new Card("VISA", 1000);
 BookingResponse soldOutBookingResponse = new BookingResponse(BookingStatus.SOLD_OUT, Optional.empty());
 when(ticketCenter.canBook()).thenReturn(false);
 BookingResponse bookingResponse = bookingService.bookTicket(johnDoe, visaCard, 500.00);
 assertEquals(soldOutBookingResponse, bookingResponse);
 }
}

• BCdotWEB: The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?.
• Joop Eggen: I would still like to see some fields final, like ticklets in TicketCenter. And BookingResponse could be a record.
• HariHaravelan: Thanks @BCdotWEB, edited the title
• Xtros: The other thing that I could see adding is argument validation and the corresponding unit tests. Most of your numbers shouldn't be negative.

Answer 1

First: there's no evidence that you've set up a sane directory structure for your project, nor any package declarations. I hope that you do have package declarations, that your main files go in src/main/java, and your test files go in src/test/java.

I hope that you're using a modern JVM. That being the case, Mockito will not run unless you pass

--add-opens java.base/java.lang=ALL-UNNAMED

I have verified this on OpenJDK 17, junit.jupiter 5.5.2 and mockito.core 2.2.2.

My opinion is that BookingResponse should be a record. The JLS describes what this record keyword means and how it works. It mentions nothing of record only being appropriate for data access objects, and the behaviour fits: it implies full immutability and simple accessors. Some people want to deny this feature to all but data access object classes and will want you to write out the boilerplate final and getter declarations.

BookingResponse.equals should type-check and type-cast in one go with a pattern, as in

 if (!(obj instanceof BookingResponse otherBookingResponse))
 return false;

Similar equals pattern use for Ticket.

Card.provider should be final, as well as all of the fields of TicketCenter.

For Card, instead of subtracting from your limit, you could add to a charge and compare the charge to a final limit. This will capture more informative state, particularly for the purposes of debugging.

• TorbenPutkonen: Can you elaborate on why you think that BookingService should be a record? It's name already implies that it's a service instead of a data object so declaring it as a record would confuse anyone who comes across it. Record is not a generic shortcut for declaring immutable classes. It has a very specific semantic meaning and I think your suggestion abuses it.