Unit Tests for BookingService.java, Overloaded Constructor in Booking…#7
Unit Tests for BookingService.java, Overloaded Constructor in Booking…#7
Conversation
…Service.java to work in testing environment. Removed unnecessary params for BookingService.java that were not utilized in current implementation. Co-authored-by: Andreas Kaiberger <andreas.kaiberger@gmail.com> Co-authored-by: Martin Karlsson <Martin.Karlsson@iths.se> Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
📝 WalkthroughWalkthroughRefactors BookingService to remove EntityManagerFactory, add a test-mode constructor using a price BigDecimal, and make guest-limit validation conditional on test mode. Updates App.main signature from Changes
Sequence Diagram(s)(skip — changes are refactor/configuration and small control-flow tweaks that do not introduce a new multi-component sequential feature) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/test/java/org/example/BookingServiceTest.java:
- Around line 47-57: The test method name
calculateTotalPriceShouldReturnNegativeValueIfStartDateIsBeforeEndDate is
misleading because the test sets start after end; rename the method in
BookingServiceTest to
calculateTotalPriceShouldReturnNegativeValueIfStartDateIsAfterEndDate (or
calculateTotalPriceShouldReturnNegativeValueIfEndDateIsBeforeStartDate) and
update any references/usages accordingly to match the new name while keeping the
test body and assertion against bookingService.calculateTotalPrice unchanged.
🧹 Nitpick comments (2)
src/main/java/org/example/BookingService.java (2)
9-21: Consider using dependency injection instead oftestModeflag.Embedding test-specific logic (
testModeflag) in production code is a design smell. SettingbookingRepository = nullintroduces latent NPE risk if the conditional logic changes.A cleaner approach: extract an interface for guest capacity validation, inject a stub/mock in tests, or pass the max-guests value directly.
♻️ Suggested alternative approach
public class BookingService { private final BookingRepository bookingRepository; private final BigDecimal priceClass; - private boolean testMode = false; + private final Integer maxGuestsOverride; public BookingService(BookingRepository bookingRepository) { this.bookingRepository = bookingRepository; priceClass = BigDecimal.valueOf(300); + this.maxGuestsOverride = null; } - public BookingService(BigDecimal priceClass) { - bookingRepository = null; + + // Test constructor - provide maxGuests explicitly instead of null repository + public BookingService(BigDecimal priceClass, int maxGuests) { + this.bookingRepository = null; this.priceClass = priceClass; - testMode = true; + this.maxGuestsOverride = maxGuests; }Then in
validateValues, usemaxGuestsOverride != null ? maxGuestsOverride : bookingRepository.getMaxGuests().
51-61: Inconsistent indentation.Lines 52-60 have extra leading spaces compared to the rest of the method. Consider normalizing the indentation for readability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/example/App.javasrc/main/java/org/example/BookingService.javasrc/test/java/org/example/BookingServiceTest.java
🔇 Additional comments (3)
src/test/java/org/example/BookingServiceTest.java (1)
12-82: Good test coverage forcalculateTotalPricevalidation paths.The tests cover the main scenarios well. Consider adding a test for the max-guests validation path (non-test mode) using a mocked
BookingRepositoryto ensure full coverage of the production code path.src/main/java/org/example/App.java (2)
24-24: LGTM!The simplified
BookingServiceinstantiation with onlyBookingRepositoryis cleaner and properly decouples the service fromEntityManagerFactory.
9-9: Java 25 supports themain()signature; however, be aware of the trade-offs.The project is configured for Java 25, which fully supports the simplified
static void main()signature withoutString[] argsand thepublicmodifier (introduced in Java 21 via JEP 445). Note that this signature removes the ability to accept command-line arguments, which may be intentional if your application doesn't need them.
| @Test | ||
| void calculateTotalPriceShouldReturnNegativeValueIfStartDateIsBeforeEndDate(){ | ||
| var guests = 1; | ||
| var start = LocalDate.now(); | ||
| var end = LocalDate.now().minusDays(1); | ||
| var bookingService = new BookingService(BigDecimal.ONE); | ||
|
|
||
| var result = bookingService.calculateTotalPrice(guests, start, end); | ||
|
|
||
| assertThat(result).isEqualTo(BigDecimal.valueOf(-1)); | ||
| } |
There was a problem hiding this comment.
Misleading test method name.
The method name calculateTotalPriceShouldReturnNegativeValueIfStartDateIsBeforeEndDate is confusing. The test actually verifies that start date is after end date (end = now minus 1 day).
Consider renaming to ...IfStartDateIsAfterEndDate or ...IfEndDateIsBeforeStartDate.
📝 Suggested fix
- void calculateTotalPriceShouldReturnNegativeValueIfStartDateIsBeforeEndDate(){
+ void calculateTotalPriceShouldReturnNegativeValueIfStartDateIsAfterEndDate(){📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void calculateTotalPriceShouldReturnNegativeValueIfStartDateIsBeforeEndDate(){ | |
| var guests = 1; | |
| var start = LocalDate.now(); | |
| var end = LocalDate.now().minusDays(1); | |
| var bookingService = new BookingService(BigDecimal.ONE); | |
| var result = bookingService.calculateTotalPrice(guests, start, end); | |
| assertThat(result).isEqualTo(BigDecimal.valueOf(-1)); | |
| } | |
| @Test | |
| void calculateTotalPriceShouldReturnNegativeValueIfStartDateIsAfterEndDate(){ | |
| var guests = 1; | |
| var start = LocalDate.now(); | |
| var end = LocalDate.now().minusDays(1); | |
| var bookingService = new BookingService(BigDecimal.ONE); | |
| var result = bookingService.calculateTotalPrice(guests, start, end); | |
| assertThat(result).isEqualTo(BigDecimal.valueOf(-1)); | |
| } |
🤖 Prompt for AI Agents
In @src/test/java/org/example/BookingServiceTest.java around lines 47 - 57, The
test method name
calculateTotalPriceShouldReturnNegativeValueIfStartDateIsBeforeEndDate is
misleading because the test sets start after end; rename the method in
BookingServiceTest to
calculateTotalPriceShouldReturnNegativeValueIfStartDateIsAfterEndDate (or
calculateTotalPriceShouldReturnNegativeValueIfEndDateIsBeforeStartDate) and
update any references/usages accordingly to match the new name while keeping the
test body and assertion against bookingService.calculateTotalPrice unchanged.
…StartDateIsBeforeEndDate() where logic was backwards but still functional. Co-authored-by: Andreas Kaiberger <andreas.kaiberger@gmail.com> Co-authored-by: Martin Karlsson <Martin.Karlsson@iths.se> Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
1 participant
…Service.java to work in testing environment.
Removed unnecessary params for BookingService.java that were not utilized in current implementation.
Summary by CodeRabbit
Refactor
Bug Fix / Validation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.