Feature/Menu#5
Conversation
…cture for booking system Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
|
Warning Rate limit exceeded@apaegs has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 31 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaced App's inline CLI with a new Menu class and delegated startup to Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App
participant Menu
participant GuestRepository
participant BookingRepository
participant BookingService
User->>App: start application
App->>Menu: new Menu(GuestRepo, BookingRepo, BookingService)
App->>Menu: menu.start()
loop user actions
User->>Menu: select option
alt create booking
Menu->>BookingService: calculateTotalPrice(numGuests,start,end)
BookingService-->>Menu: totalPrice / -1 on invalid
Menu->>GuestRepository: findByEmail / create guest(s)
GuestRepository-->>Menu: guest(s)
Menu->>BookingRepository: getEmptyRooms(start,end,guests)
BookingRepository-->>Menu: rooms
Menu->>BookingRepository: create(emails,start,end,guests,totalPrice)
BookingRepository-->>Menu: success/failure
Menu-->>User: result
else view/cancel bookings
Menu->>BookingRepository: findAll()/findByGuest()/findGuestsByBooking()/delete(id)
BookingRepository-->>Menu: results/confirmation
Menu-->>User: display result
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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. 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/main/java/org/example/App.java:
- Around line 14-16: App.java currently hard-codes DB connection details via the
.jdbcUrl, .jdbcUsername and .jdbcPassword builder calls; replace these
hard-coded values with configurable sources (prefer environment variables or a
configuration/properties file) by reading
System.getenv("DB_URL")/System.getenv("DB_USER")/System.getenv("DB_PASS") or a
loaded Properties object and passing those values into the
.jdbcUrl/.jdbcUsername/.jdbcPassword calls, providing sensible fallbacks or
failing fast with clear errors if values are missing.
🧹 Nitpick comments (2)
src/main/java/org/example/App.java (1)
8-8: Remove unused import.The
java.awt.*import is not used anywhere in this file.♻️ Proposed fix
-import java.awt.*; import java.util.Scanner;src/main/java/org/example/Menu.java (1)
7-7: Consider adding error handling for Scanner operations.While the Scanner correctly wraps System.in and is intentionally not closed, the
start()method should handle potentialNoSuchElementExceptionor other Scanner exceptions to prevent unexpected crashes.💡 Example error handling
public void start() { while (true) { - System.out.println("\n=== Booking System ==="); - System.out.println("1) Create booking"); - System.out.println("2) View all bookings"); - System.out.println("3) View all bookings by guest"); - System.out.println("4) View all guests by booking"); - System.out.println("5) Cancel booking"); - System.out.println("0) Exit"); - System.out.print("Choose: "); - - String choice = scanner.nextLine(); + try { + System.out.println("\n=== Booking System ==="); + System.out.println("1) Create booking"); + System.out.println("2) View all bookings"); + System.out.println("3) View all bookings by guest"); + System.out.println("4) View all guests by booking"); + System.out.println("5) Cancel booking"); + System.out.println("0) Exit"); + System.out.print("Choose: "); + + String choice = scanner.nextLine(); + // ... rest of switch logic + } catch (Exception e) { + System.err.println("Error reading input: " + e.getMessage()); + } + } +}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/example/App.javasrc/main/java/org/example/Menu.java
🔇 Additional comments (4)
src/main/java/org/example/App.java (1)
27-28: Clean delegation to Menu class.The refactoring successfully separates CLI concerns into the Menu class, improving maintainability and single responsibility.
src/main/java/org/example/Menu.java (3)
12-15: Good use of dependency injection.The constructor properly injects repository dependencies, enabling testability and loose coupling.
17-43: Clean menu structure with proper exit handling.The infinite loop with a clear exit point (case "0") and switch expressions provide a clean, modern menu implementation.
46-66: Stub methods appropriately marked with TODOs.All placeholder methods are clearly marked with TODO comments describing their intended functionality. This provides good guidance for future implementation.
| .jdbcUrl("jdbc:mysql://localhost:3306/booking_db") | ||
| .jdbcUsername("root") | ||
| .jdbcPassword("root") |
There was a problem hiding this comment.
Externalize database credentials and configuration.
Hard-coded database credentials ("root"/"root") and connection URLs pose security and portability risks. Consider externalizing these to environment variables or a configuration file.
💡 Example approach using environment variables
- .jdbcUrl("jdbc:mysql://localhost:3306/booking_db")
- .jdbcUsername("root")
- .jdbcPassword("root")
+ .jdbcUrl(System.getenv("DB_URL"))
+ .jdbcUsername(System.getenv("DB_USER"))
+ .jdbcPassword(System.getenv("DB_PASSWORD"))📝 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.
| .jdbcUrl("jdbc:mysql://localhost:3306/booking_db") | |
| .jdbcUsername("root") | |
| .jdbcPassword("root") | |
| .jdbcUrl(System.getenv("DB_URL")) | |
| .jdbcUsername(System.getenv("DB_USER")) | |
| .jdbcPassword(System.getenv("DB_PASSWORD")) |
🤖 Prompt for AI Agents
In @src/main/java/org/example/App.java around lines 14 - 16, App.java currently
hard-codes DB connection details via the .jdbcUrl, .jdbcUsername and
.jdbcPassword builder calls; replace these hard-coded values with configurable
sources (prefer environment variables or a configuration/properties file) by
reading
System.getenv("DB_URL")/System.getenv("DB_USER")/System.getenv("DB_PASS") or a
loaded Properties object and passing those values into the
.jdbcUrl/.jdbcUsername/.jdbcPassword calls, providing sensible fallbacks or
failing fast with clear errors if values are missing.
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/java/org/example/Menu.java:
- Around line 7-15: Menu currently holds a Scanner (field scanner) that is never
closed; implement AutoCloseable on the Menu class and add a close() method that
closes the Scanner instance (scanner.close()), then update usages (e.g., where
Menu is constructed and started in App.java) to use try-with-resources so Menu
is auto-closed; ensure the Menu constructor and start() semantics remain
unchanged but the class signature implements AutoCloseable and the scanner field
is closed in close().
🧹 Nitpick comments (2)
src/main/java/org/example/Menu.java (2)
17-48: Consider more specific exception handling.The catch block handles all exceptions generically, which keeps the menu running but might mask programming errors. For a CLI application this is acceptable, but you could differentiate between expected input errors and unexpected runtime exceptions.
♻️ Optional: More granular exception handling
while (true) { try { System.out.println("\n=== Booking System ==="); System.out.println("1) Create booking"); System.out.println("2) View all bookings"); System.out.println("3) View all bookings by guest"); System.out.println("4) View all guests by booking"); System.out.println("5) Cancel booking"); System.out.println("0) Exit"); System.out.print("Choose: "); String choice = scanner.nextLine(); switch (choice) { case "1" -> createBooking(); case "2" -> printBookings(); case "3" -> printBookingsByGuest(); case "4" -> printGuestsByBooking(); case "5" -> removeBooking(); case "0" -> { System.out.println("Goodbye!"); return; } default -> System.out.println("Invalid choice"); } + } catch (java.util.NoSuchElementException | IllegalStateException e) { + System.err.println("Error reading input: " + e.getMessage()); - } catch (Exception e) { - System.err.println("Error reading input: " + e.getMessage()); + } catch (Exception e) { + System.err.println("Unexpected error: " + e.getMessage()); + e.printStackTrace(); } }
51-71: Stub methods are appropriately marked for future implementation.The placeholder methods with //todo comments clearly outline the intended functionality. The structure provides a good foundation for implementing the booking operations.
Do you want me to help generate implementation code for any of these methods, or open issues to track each implementation task?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/example/App.javasrc/main/java/org/example/Menu.java
🔇 Additional comments (2)
src/main/java/org/example/App.java (1)
15-28: LGTM! Clean refactoring to Menu-driven architecture.The extraction of CLI logic into the Menu class is well-executed. The EntityManagerFactory is properly managed with try-with-resources, and the Menu receives the necessary repository dependencies.
src/main/java/org/example/Menu.java (1)
1-73: Verification confirmed: removed App methods are not called elsewhere.No references to the removed App methods (printBookings, printBookingsByGuest, printGuestsByBooking, createBooking, removeBooking) exist in the codebase. App.java correctly instantiates Menu and delegates to menu.start().
| private final Scanner scanner = new Scanner(System.in); | ||
|
|
||
| private final GuestRepository guestRepo; | ||
| private final BookingRepository bookingRepo; | ||
|
|
||
| public Menu(GuestRepository guestRepo, BookingRepository bookingRepo) { | ||
| this.guestRepo = guestRepo; | ||
| this.bookingRepo = bookingRepo; | ||
| } |
There was a problem hiding this comment.
Scanner resource leak - implement AutoCloseable.
The Scanner is never closed, leaking the System.in resource when the menu exits. Menu should implement AutoCloseable and close the Scanner, or App.java should call a cleanup method.
🔒 Proposed fix: Implement AutoCloseable
-public class Menu {
+public class Menu implements AutoCloseable {
private final Scanner scanner = new Scanner(System.in);
private final GuestRepository guestRepo;
private final BookingRepository bookingRepo;
public Menu(GuestRepository guestRepo, BookingRepository bookingRepo) {
this.guestRepo = guestRepo;
this.bookingRepo = bookingRepo;
}
+
+ @Override
+ public void close() {
+ scanner.close();
+ }Then update App.java to use try-with-resources:
try (EntityManagerFactory emf = cfg.createEntityManagerFactory();
Menu menu = new Menu(new GuestRepository(emf),
new BookingRepository(emf, new GuestRepository(emf)))) {
menu.start();
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/main/java/org/example/Menu.java around lines 7 - 15, Menu currently
holds a Scanner (field scanner) that is never closed; implement AutoCloseable on
the Menu class and add a close() method that closes the Scanner instance
(scanner.close()), then update usages (e.g., where Menu is constructed and
started in App.java) to use try-with-resources so Menu is auto-closed; ensure
the Menu constructor and start() semantics remain unchanged but the class
signature implements AutoCloseable and the scanner field is closed in close().
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
…in Menu.java Co-authored-by: Rickard Ankar <rickard.ankar@iths.se>
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
# Conflicts: # src/main/java/org/example/App.java
Implement priceCalculation() Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/example/BookingRepository.java (1)
104-127:getFirst()will throwNoSuchElementExceptionif no rooms are available.Line 106 calls
getEmptyRooms(...).getFirst()without checking if the list is empty. If no rooms match the criteria, this will throw an exception. The caller (Menu.createBooking) does checkgetEmptyRoomsfirst (line 165), but this method doesn't enforce that contract.🐛 Suggested defensive fix
public boolean create(List<String> emailList, LocalDate startDate, LocalDate endDate, int numberOfGuests, BigDecimal totalPrice){ return emf.callInTransaction(em -> { - Room room = getEmptyRooms(startDate, endDate, numberOfGuests).getFirst(); + List<Room> emptyRooms = getEmptyRooms(startDate, endDate, numberOfGuests); + if (emptyRooms.isEmpty()) { + return false; + } + Room room = emptyRooms.getFirst();
🤖 Fix all issues with AI agents
In @src/main/java/org/example/Menu.java:
- Around line 87-92: The guest existence check prints a message but does not
stop execution, so bookingRepo.getBookingInfoByGuest(email) will run for a
non-existent guest; update the control flow in the method containing
guestRepo.guestExist(email) (the block that assigns var bookings) to return (or
otherwise exit/skip the subsequent logic) immediately after printing the "No
guest found" message when guestRepo.guestExist(email) is false, ensuring
bookingRepo.getBookingInfoByGuest(email) is only called for existing guests.
- Line 170: The print statement in Menu (the System.out.println that builds the
guest label using variable i) suffers from operator precedence so `"guest #" + i
+ 1` concatenates i as a string then adds 1; fix it by grouping the arithmetic
or formatting the value—e.g., build the string as "guest #" + (i + 1) or use
String.format("guest #%d", i + 1) where the current System.out.println occurs so
guest numbering prints correctly.
- Around line 162-196: The bookingRepo.create(...) call is executed even when no
rooms are available, causing an exception; wrap the booking creation logic so
that bookingRepo.create(emails, start, end, guests, totalPrice) is only invoked
inside the existing if (!bookingRepo.getEmptyRooms(start, end,
guests).isEmpty()) block after emails is populated, and add an else branch that
prints a clear "No rooms available" (or similar) message and returns/aborts the
booking flow; ensure the emails list and related guest prompt loop remain
scoped/used only when the room-availability condition is true.
🧹 Nitpick comments (6)
src/main/java/org/example/BookingService.java (3)
10-18: Unusedemffield.The
emffield is injected via the constructor but never used within the class. Consider removing it if it's not needed, or document its intended future use.♻️ Suggested fix
public class BookingService { - private EntityManagerFactory emf; private BookingRepository bookingRepository; private BigDecimal priceClass; - public BookingService (EntityManagerFactory emf, BookingRepository bookingRepository) { - this.emf = emf; + public BookingService (BookingRepository bookingRepository) { this.bookingRepository = bookingRepository; priceClass = BigDecimal.valueOf(300); }Note: This would require updating
App.javaas well.
38-52: Validation method has side effects (console output).
validateValuesprints error messages directly toSystem.out, coupling validation logic with presentation. This makes the service harder to test and reuse. Consider returning a validation result object or throwing exceptions, letting the caller handle user messaging.
27-36: Consider usingOptionalinstead of sentinel value-1.Returning
-1to indicate validation failure is error-prone and requires callers to remember to check for it. AnOptional<BigDecimal>or a dedicated result type would be more explicit and type-safe.src/main/java/org/example/Menu.java (3)
58-75: Consider early return for empty bookings.The empty check prints a message but still continues to the
forEach. While harmless (empty list iteration does nothing), an earlyreturnwould be clearer.♻️ Suggested improvement
// Validate that bookings is not empty if (bookings.isEmpty()) { System.out.println("No bookings found."); + return; }
137-153: Unhandled parse exceptions for user input.
LocalDate.parse()(lines 139, 142) andInteger.parseInt()(line 151) can throw exceptions on malformed input. While the outercatchinstart()handles this, users will see cryptic messages. Consider adding input validation with user-friendly error messages.
217-232: Inverted boolean semantics invalidateBookingId.The method returns
truewhen validation fails, which is counterintuitive. Callers useif (validateBookingId(bookingId)) return;, which works but is confusing. Consider renaming toisInvalidBookingId()or inverting the logic.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/org/example/App.javasrc/main/java/org/example/BookingRepository.javasrc/main/java/org/example/BookingService.javasrc/main/java/org/example/Menu.java
🔇 Additional comments (4)
src/main/java/org/example/BookingRepository.java (2)
143-153: LGTM!The
getMaxGuestsmethod correctly handles the case where no rooms exist by usingOptional.ofNullableand returning0as a safe default.
20-40: LGTM!The parameter type change from
longtointis appropriate for guest counts and consistent with the rest of the codebase.src/main/java/org/example/Menu.java (1)
24-55: LGTM!The main menu loop structure is clear, with proper exit handling and exception recovery for CLI robustness.
src/main/java/org/example/App.java (1)
21-28: LGTM!Clean separation of concerns - App handles configuration and dependency wiring, then delegates CLI control to
Menu. The wiring matches the updatedBookingServiceconstructor signature.
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/main/java/org/example/Menu.java:
- Around line 96-108: The check for an empty bookings list prints "No bookings
found." but doesn't return, so the method continues to the bookings.forEach
call; add an early return after the System.out.println("No bookings found.") so
the method exits immediately when bookings.isEmpty() is true (matching the
behavior of printBookings()), keeping the rest of the method (the
bookings.forEach that prints each Booking b with b.id(), b.roomNumber(),
b.startDate(), b.endDate(), b.totalPrice()) unchanged.
- Around line 139-155: User input parsing can throw DateTimeParseException and
NumberFormatException and the current return on invalid date exits the method
instead of re-prompting; wrap the LocalDate.parse calls and Integer.parseInt
call in try/catch blocks (catch DateTimeParseException and
NumberFormatException), print a friendly error and continue the loop rather than
returning, and only call bookingService.calculateTotalPrice when parsing
succeeds; adjust the do/while loop to use continue to restart on invalid input
and ensure totalPrice is only used after successful parsing so the loop
condition (totalPrice.signum()) is safe.
🧹 Nitpick comments (2)
src/main/java/org/example/Menu.java (2)
186-189: Redundant condition.The
else if (guestRepo.guestExist(email))check on line 187 is always true at this point since theifon line 176 already established the guest exists when this branch is reached.Proposed simplification
if (isValidGuestNames(firstName, lastName)) { guestRepo.create(firstName, lastName, email); guestExists = true; } - } else if (guestRepo.guestExist(email)){ + } else { + // Guest already exists guestExists = true; }
195-198: Condition!email.isEmpty()is always true here.At line 195,
while (!guestExists)loop only exits after a non-empty email is processed (checked at line 175). This condition can be removed for clarity.Proposed simplification
- if (!email.isEmpty()) { - bookingRepo.create(emails, start, end, guests, totalPrice); - System.out.println("Booking created."); - } + bookingRepo.create(emails, start, end, guests, totalPrice); + System.out.println("Booking created.");
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/example/Menu.java
🔇 Additional comments (6)
src/main/java/org/example/Menu.java (6)
10-22: LGTM! Clean dependency injection.The constructor properly injects dependencies as final fields. The
ScannerwrappingSystem.inis intentionally left unclosed since closing it would closeSystem.inglobally, which is the correct approach for CLI applications.
24-55: LGTM! Well-structured menu loop.The menu loop is clean with proper exit handling. The broad exception catch provides resilience for a CLI application while still logging errors.
58-76: LGTM!Clean implementation with proper empty-state handling.
111-132: LGTM!Good validation flow and proper empty-state handling with early return.
208-221: LGTM!Proper validation and exception handling for the cancellation flow.
223-246: LGTM!Validation helpers are straightforward. Minor inconsistency:
isInvalidBookingIdisstaticwhileisValidGuestNamesis not (though neither uses instance state). Consider making them consistent, but this is a nitpick.
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/main/java/org/example/Menu.java:
- Around line 179-182: The code lowers guest names by calling .toLowerCase() on
input when assigning firstName and lastName; instead preserve user casing or
apply title-case: remove the .toLowerCase() calls on the scanner results (keep
.trim()) or replace them with a capitalization routine that uppercases the first
character and lowercases the rest before assigning to firstName and lastName;
update any downstream uses that assumed lowercase if necessary (look for the
variables firstName and lastName in Menu.java).
- Around line 142-157: The loop can call
bookingService.calculateTotalPrice(guests, start, end) with null start/end when
LocalDate.parse or Integer.parse fails; in the catch block for
NumberFormatException | DateTimeParseException, skip the calculation and prompt
again by using continue (or otherwise re-check that start, end and guests are
non-null/valid before calling bookingService.calculateTotalPrice), so modify the
do/while to continue on parse errors and only call
bookingService.calculateTotalPrice when start and end are successfully parsed
and guests is a valid positive integer.
- Around line 159-169: Move the room-availability check so it runs before
printing or confirming the total price: call bookingRepo.getEmptyRooms(start,
end, guests) immediately after date/guest input and before the
System.out.println that displays totalPrice and before prompting the scanner for
confirmation, returning or informing the user if the list is empty; also change
the rounding behavior on totalPrice from RoundingMode.CEILING to
RoundingMode.HALF_UP to avoid always rounding up (update the
totalPrice.setScale(...) call accordingly).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/example/Menu.java
🔇 Additional comments (5)
src/main/java/org/example/Menu.java (5)
11-24: LGTM - Clean dependency injection setup.Constructor injection with final fields is the right pattern here. The
Scanner(System.in)is intentionally not closed since closing it would also closeSystem.in, which would break subsequent input operations.
26-57: LGTM - Well-structured menu loop.The switch expression provides clean dispatch, and the broad exception handler is acceptable for a CLI application to prevent crashes from unexpected input errors.
60-112: LGTM - Good validation and output handling.Both methods properly check for empty results before iterating and provide clear user feedback. The email validation in
printBookingsByGuest()correctly checks existence before querying bookings.
114-135: LGTM - Clean implementation.Proper validation delegation and clear error messaging for edge cases.
208-246: LGTM - Clean validation helpers and error handling.The
isInvalidBookingId()method consolidates validation logic well, andremoveBooking()properly handles repository exceptions with user-friendly error messages.
| System.out.print("First name: "); | ||
| String firstName = scanner.nextLine().trim().toLowerCase(); | ||
| System.out.print("Last name: "); | ||
| String lastName = scanner.nextLine().trim().toLowerCase(); |
There was a problem hiding this comment.
Lowercasing guest names may be undesirable.
Converting firstName and lastName to lowercase results in names like "john smith" instead of "John Smith". Consider preserving the case or applying proper title case instead.
Suggested fix: preserve original case or capitalize
System.out.print("First name: ");
- String firstName = scanner.nextLine().trim().toLowerCase();
+ String firstName = scanner.nextLine().trim();
System.out.print("Last name: ");
- String lastName = scanner.nextLine().trim().toLowerCase();
+ String lastName = scanner.nextLine().trim();📝 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.
| System.out.print("First name: "); | |
| String firstName = scanner.nextLine().trim().toLowerCase(); | |
| System.out.print("Last name: "); | |
| String lastName = scanner.nextLine().trim().toLowerCase(); | |
| System.out.print("First name: "); | |
| String firstName = scanner.nextLine().trim(); | |
| System.out.print("Last name: "); | |
| String lastName = scanner.nextLine().trim(); |
🤖 Prompt for AI Agents
In @src/main/java/org/example/Menu.java around lines 179 - 182, The code lowers
guest names by calling .toLowerCase() on input when assigning firstName and
lastName; instead preserve user casing or apply title-case: remove the
.toLowerCase() calls on the scanner results (keep .trim()) or replace them with
a capitalization routine that uppercases the first character and lowercases the
rest before assigning to firstName and lastName; update any downstream uses that
assumed lowercase if necessary (look for the variables firstName and lastName in
Menu.java).
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/java/org/example/Menu.java:
- Around line 142-162: The catch block handling NumberFormatException |
DateTimeParseException must stop the current loop iteration so null/invalid
values aren't passed to bookingRepo.getEmptyRooms(start, end, guests); update
the try/catch in the do/while around LocalDate.parse and Integer.parseInt to add
a continue after printing the error message so the loop restarts when parsing
fails; ensure you reference the existing symbols LocalDate.parse,
bookingRepo.getEmptyRooms(start, end, guests), and the variables
start/end/guests when making this change.
🧹 Nitpick comments (2)
src/main/java/org/example/Menu.java (2)
174-205: Redundant room availability check.The room availability check at line 174 duplicates the check already performed at line 156. Since significant user interaction occurs between these checks, consider either:
- Removing the redundant check if concurrency isn't a concern, or
- Keeping only the check at line 174 (right before booking creation) and removing line 156-159 to avoid the TOCTOU gap.
♻️ Option 1: Remove redundant check (simpler)
List<String> emails = new ArrayList<>(); String email = ""; - if (!bookingRepo.getEmptyRooms(start, end, guests).isEmpty()) { - for (int i = 0; i < guests; i++) { + for (int i = 0; i < guests; i++) { // ... guest collection logic - } + } - bookingRepo.create(emails, start, end, guests, totalPrice); - System.out.println("Booking created."); + if (bookingRepo.getEmptyRooms(start, end, guests).isEmpty()) { + System.out.println("No rooms available anymore."); + return; + } - } + bookingRepo.create(emails, start, end, guests, totalPrice); + System.out.println("Booking created.");
242-248: Consider making this method static for consistency.This method doesn't use any instance state, similar to
isInvalidBookingId. Making itstaticwould be consistent with the other validation helper.♻️ Suggested change
- private boolean isValidGuestNames(String firstName, String lastName){ + private static boolean isValidGuestNames(String firstName, String lastName) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/example/Menu.java
🔇 Additional comments (7)
src/main/java/org/example/Menu.java (7)
11-24: LGTM!Constructor properly uses dependency injection for repositories and service. The
ScannerwrappingSystem.inis correctly kept as a field without explicit closing, since closing it would closeSystem.inand break further input in the CLI application.
26-57: LGTM!The menu loop structure is clean with proper exit handling. The broad exception catch is acceptable for a CLI application to prevent unexpected crashes while still logging errors.
60-78: LGTM!Clean implementation with proper handling of empty bookings list before iterating.
80-112: LGTM!Proper validation sequence: checks for empty email, verifies guest existence, then handles empty booking results gracefully.
114-135: LGTM!Clean implementation with proper validation and empty result handling.
210-223: LGTM!Proper validation and exception handling for the cancellation flow.
225-240: LGTM!Clear validation logic with appropriate error messages for both empty and non-numeric booking IDs.
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/org/example/Menu.java (3)
167-171: Consider explicit confirmation handling.Currently, only
"n"cancels the booking; any other input (including typos like"yes","ok", or accidental enters) proceeds with the booking. For a more defensive approach, require explicit"y"confirmation.Suggested fix
System.out.println("Do you want to continue? (Y/N)"); String input = scanner.nextLine().toLowerCase(); - if (input.equals("n")) { + if (!input.equals("y")) { + System.out.println("Booking cancelled."); return; }
176-176: Redundant room availability check.This check duplicates line 158. If no rooms were available, the method would have returned at line 160. However, in a concurrent environment, this re-check provides a safeguard against rooms being booked between the price display and booking creation. Consider whether this is intentional (and if so, add a comment) or can be removed.
187-189: Guest names are lowercased—is this intentional?Converting
firstNameandlastNameto lowercase may not align with user expectations, as names typically preserve their original casing (e.g., "John Doe" vs "john doe").Suggested fix
System.out.print("First name: "); - String firstName = scanner.nextLine().trim().toLowerCase(); + String firstName = scanner.nextLine().trim(); System.out.print("Last name: "); - String lastName = scanner.nextLine().trim().toLowerCase(); + String lastName = scanner.nextLine().trim();
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/example/Menu.java
🔇 Additional comments (8)
src/main/java/org/example/Menu.java (8)
12-25: LGTM!The class structure and dependency injection via constructor are clean. The
INVALIDsentinel constant is a reasonable approach for tracking uninitialized state.
27-58: LGTM!The main menu loop is well-structured with clear options and proper exit handling. The switch expression syntax is clean.
61-79: LGTM!Clean implementation with proper empty-check before iteration.
81-113: LGTM!Good validation sequence: empty check → guest existence → bookings existence. Clear error messages for each case.
115-136: LGTM!Proper validation using the helper method and clear output formatting.
212-225: LGTM!Proper validation and error handling for the cancel operation.
227-242: LGTM!Validation logic is correct. Minor note: this is
staticwhileisValidGuestNamesis an instance method—consider making them consistent, though it's not critical.
244-250: LGTM!Simple and effective validation. The upstream
trim()calls ensure whitespace-only inputs are properly rejected.
| do { | ||
| try { | ||
| System.out.print("Start date (YYYY-MM-DD): "); | ||
| start = LocalDate.parse(scanner.nextLine()); | ||
|
|
||
| System.out.print("End date (YYYY-MM-DD): "); | ||
| end = LocalDate.parse(scanner.nextLine()); | ||
|
|
||
| System.out.print("Number of guests: "); | ||
| guests = Integer.parseInt(scanner.nextLine()); | ||
| } catch (NumberFormatException | DateTimeParseException e) { | ||
| System.out.println("Invalid Date format/Number of guests needs to be a whole number."); | ||
| continue; | ||
| } | ||
|
|
||
| if (bookingRepo.getEmptyRooms(start, end, guests).isEmpty()) { | ||
| System.out.println("No rooms available for the selected dates and guest count."); | ||
| return; | ||
| } | ||
|
|
||
| totalPrice = bookingService.calculateTotalPrice(guests, start, end); | ||
| } while (Objects.requireNonNull(totalPrice).signum() != 1); |
There was a problem hiding this comment.
Potential infinite loop if price calculation returns zero or negative.
The do-while condition Objects.requireNonNull(totalPrice).signum() != 1 loops until the price is positive. However, if calculateTotalPrice consistently returns a non-positive value (e.g., due to invalid business rules or edge cases), the user has no way to exit this loop—there's no prompt or break mechanism.
Suggested fix
} catch (NumberFormatException | DateTimeParseException e) {
System.out.println("Invalid Date format/Number of guests needs to be a whole number.");
continue;
}
if (bookingRepo.getEmptyRooms(start, end, guests).isEmpty()) {
System.out.println("No rooms available for the selected dates and guest count.");
return;
}
totalPrice = bookingService.calculateTotalPrice(guests, start, end);
- } while (Objects.requireNonNull(totalPrice).signum() != 1);
+ if (totalPrice == null || totalPrice.signum() != 1) {
+ System.out.println("Could not calculate a valid price. Please try different dates or guest count.");
+ System.out.print("Try again? (Y/N): ");
+ if (scanner.nextLine().trim().equalsIgnoreCase("n")) {
+ return;
+ }
+ }
+ } while (totalPrice == null || totalPrice.signum() != 1);Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @src/main/java/org/example/Menu.java:
- Around line 188-190: The code currently forces guest names to lowercase by
calling .toLowerCase() on scanner input; remove the .toLowerCase() calls so
firstName and lastName are assigned with scanner.nextLine().trim() to preserve
user casing, or alternatively apply a proper capitalization routine (e.g.,
title-case each word) before storing; update the two assignments for firstName
and lastName (the lines currently using .toLowerCase()) accordingly and ensure
any downstream usage expects the preserved/formatted casing.
- Around line 167-172: The confirmation logic in Menu.java currently treats
anything other than "n" as confirmation; update the block that reads from
scanner (the variable scanner) to explicitly accept "y" (and "Y") as
confirmation, treat "n" (and "N") as cancellation, and loop to reprompt on any
other invalid input until a valid response is provided; replace the current
single-read check (input.equals("n")) with a small loop that normalizes input
(toLowerCase or equalsIgnoreCase) and only proceeds or returns when "y" or "n"
is entered.
- Around line 163-164: The do-while loop using
Objects.requireNonNull(totalPrice).signum() != 1 in Menu (around totalPrice and
bookingService.calculateTotalPrice) can trap the user in an infinite loop when
calculateTotalPrice returns zero/negative; change the flow to explicitly handle
three cases after calling bookingService.calculateTotalPrice: if the result is
null or negative treat it as a service error (log/show a clear message and
either break/return to the main menu or throw a handled exception), if the
result is zero treat it as a valid but free booking (inform the user and
continue), and if positive proceed; also add a user-visible escape path or retry
limit so the UI does not loop forever.
🧹 Nitpick comments (5)
src/main/java/org/example/Menu.java (5)
21-25: Constructor lacks null validation for dependencies.Consider adding null checks or using
Objects.requireNonNull()for the injected dependencies to fail fast if misconfigured.♻️ Optional improvement
public Menu(GuestRepository guestRepo, BookingRepository bookingRepo, BookingService bookingService) { - this.guestRepo = guestRepo; - this.bookingRepo = bookingRepo; - this.bookingService = bookingService; + this.guestRepo = Objects.requireNonNull(guestRepo, "guestRepo cannot be null"); + this.bookingRepo = Objects.requireNonNull(bookingRepo, "bookingRepo cannot be null"); + this.bookingService = Objects.requireNonNull(bookingService, "bookingService cannot be null"); }
53-55: Overly broad exception handling.Catching
Exceptionis too broad and may mask programming errors. Consider catching only expected exceptions likeNoSuchElementExceptionfrom the scanner.
83-89: Consider trimming input and checking for blank strings.Using
isEmpty()won't catch whitespace-only input. Consider usingisBlank()(Java 11+) or trimming the input first for consistency with other methods in this class.♻️ Suggested improvement
System.out.print("Guest email: "); - String email = scanner.nextLine(); + String email = scanner.nextLine().trim(); // Validate if email is not empty - if (email.isEmpty()) { + if (email.isBlank()) { System.out.println("Email cannot be empty"); return; }
213-226: Consider adding confirmation before cancelling booking.The booking is cancelled immediately without user confirmation. For destructive operations, a Y/N prompt (similar to the one in
createBooking) would prevent accidental cancellations.
245-251: Inconsistent method modifiers and validation approach.
isInvalidBookingIdisstaticwhileisValidGuestNamesis an instance method, though neither uses instance state. Also,isValidGuestNamesusesisEmpty()which doesn't catch whitespace-only names. Consider usingisBlank()for consistency.♻️ Suggested fix
- private boolean isValidGuestNames(String firstName, String lastName){ - if (firstName.isEmpty() || lastName.isEmpty()) { + private static boolean isValidGuestNames(String firstName, String lastName) { + if (firstName.isBlank() || lastName.isBlank()) { System.out.println("First and last name can't be blank."); return false; } return true; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/example/Menu.java
🔇 Additional comments (2)
src/main/java/org/example/Menu.java (2)
61-79: LGTM!The method correctly handles the empty bookings case and uses a clean lambda expression for output formatting.
115-136: LGTM!The method properly validates the booking ID and handles the case where no guests are found.
| totalPrice = bookingService.calculateTotalPrice(guests, start, end); | ||
| } while (Objects.requireNonNull(totalPrice).signum() != 1); |
There was a problem hiding this comment.
Confusing loop condition and potential infinite loop.
The do-while condition Objects.requireNonNull(totalPrice).signum() != 1 will loop indefinitely if calculateTotalPrice returns zero or a negative value. If this indicates a service-layer error rather than invalid user input, the user has no way to exit. Consider clarifying when a non-positive price can occur and providing an escape path.
🐛 Suggested fix
totalPrice = bookingService.calculateTotalPrice(guests, start, end);
- } while (Objects.requireNonNull(totalPrice).signum() != 1);
+ if (totalPrice == null || totalPrice.signum() != 1) {
+ System.out.println("Could not calculate a valid price. Please try again or enter 'q' to cancel.");
+ String retry = scanner.nextLine().trim().toLowerCase();
+ if (retry.equals("q")) {
+ return;
+ }
+ }
+ } while (totalPrice == null || totalPrice.signum() != 1);🤖 Prompt for AI Agents
In @src/main/java/org/example/Menu.java around lines 163 - 164, The do-while
loop using Objects.requireNonNull(totalPrice).signum() != 1 in Menu (around
totalPrice and bookingService.calculateTotalPrice) can trap the user in an
infinite loop when calculateTotalPrice returns zero/negative; change the flow to
explicitly handle three cases after calling bookingService.calculateTotalPrice:
if the result is null or negative treat it as a service error (log/show a clear
message and either break/return to the main menu or throw a handled exception),
if the result is zero treat it as a valid but free booking (inform the user and
continue), and if positive proceed; also add a user-visible escape path or retry
limit so the UI does not loop forever.
| System.out.println("Do you want to continue? (Y/N)"); | ||
| String input = scanner.nextLine().toLowerCase(); | ||
| if (input.equals("n")) { | ||
| System.out.println("Booking cancelled."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Confirmation prompt accepts any input except "n" as affirmative.
Only "n" cancels the booking; any other input (including typos like "no", "N", or accidental keypresses) proceeds. Consider explicitly checking for "y" and reprompting on invalid input.
💡 Suggested improvement
System.out.println("Do you want to continue? (Y/N)");
- String input = scanner.nextLine().toLowerCase();
- if (input.equals("n")) {
- System.out.println("Booking cancelled.");
- return;
+ String input = scanner.nextLine().trim().toLowerCase();
+ while (!input.equals("y") && !input.equals("n")) {
+ System.out.println("Please enter Y or N:");
+ input = scanner.nextLine().trim().toLowerCase();
+ }
+ if (input.equals("n")) {
+ System.out.println("Booking cancelled.");
+ return;
}📝 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.
| System.out.println("Do you want to continue? (Y/N)"); | |
| String input = scanner.nextLine().toLowerCase(); | |
| if (input.equals("n")) { | |
| System.out.println("Booking cancelled."); | |
| return; | |
| } | |
| System.out.println("Do you want to continue? (Y/N)"); | |
| String input = scanner.nextLine().trim().toLowerCase(); | |
| while (!input.equals("y") && !input.equals("n")) { | |
| System.out.println("Please enter Y or N:"); | |
| input = scanner.nextLine().trim().toLowerCase(); | |
| } | |
| if (input.equals("n")) { | |
| System.out.println("Booking cancelled."); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In @src/main/java/org/example/Menu.java around lines 167 - 172, The confirmation
logic in Menu.java currently treats anything other than "n" as confirmation;
update the block that reads from scanner (the variable scanner) to explicitly
accept "y" (and "Y") as confirmation, treat "n" (and "N") as cancellation, and
loop to reprompt on any other invalid input until a valid response is provided;
replace the current single-read check (input.equals("n")) with a small loop that
normalizes input (toLowerCase or equalsIgnoreCase) and only proceeds or returns
when "y" or "n" is entered.
| if (!bookingRepo.getEmptyRooms(start, end, guests).isEmpty()) { | ||
| for (int i = 0; i < guests; i++) { | ||
| boolean guestExists = false; | ||
| while (!guestExists) { | ||
| // verify not empty | ||
| System.out.println("Enter details for guest #" + (i + 1)); | ||
| System.out.print("Email: "); | ||
| email = scanner.nextLine().trim().toLowerCase(); | ||
| if (!email.isEmpty()) { | ||
| if (!guestRepo.guestExist(email)) { | ||
| System.out.print("First name: "); | ||
| String firstName = scanner.nextLine().trim().toLowerCase(); | ||
| System.out.print("Last name: "); | ||
| String lastName = scanner.nextLine().trim().toLowerCase(); | ||
|
|
||
| if (isValidGuestNames(firstName, lastName)) { | ||
| guestRepo.create(firstName, lastName, email); | ||
| guestExists = true; | ||
| } | ||
|
|
||
| } else{ | ||
| guestExists = true; | ||
| } | ||
| } | ||
| } | ||
| emails.add(email); | ||
| } | ||
|
|
||
| bookingRepo.create(emails, start, end, guests, totalPrice); | ||
| System.out.println("Booking created."); | ||
|
|
||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/java/org/example/Menu.java | sed -n '150,215p'Repository: ithsjava25/project-jpa-grupp-6-g
Length of output: 3215
🏁 Script executed:
find . -type f -name "*BookingRepository*" | head -5Repository: ithsjava25/project-jpa-grupp-6-g
Length of output: 126
🏁 Script executed:
rg -n "getEmptyRooms|bookingRepo\.create|@Transactional|synchronized" --type=java -A 3 -B 3Repository: ithsjava25/project-jpa-grupp-6-g
Length of output: 2953
🏁 Script executed:
cat -n src/main/java/org/example/BookingRepository.java | sed -n '104,130p'Repository: ithsjava25/project-jpa-grupp-6-g
Length of output: 1362
🏁 Script executed:
wc -l src/main/java/org/example/BookingRepository.javaRepository: ithsjava25/project-jpa-grupp-6-g
Length of output: 128
🏁 Script executed:
cat -n src/main/java/org/example/BookingRepository.javaRepository: ithsjava25/project-jpa-grupp-6-g
Length of output: 6764
Remove redundant availability check at line 177; implement atomic room booking.
The availability check at line 177 is redundant—if no rooms were available at line 158, the method returns early at line 160, making the second check always true. More critically, the time gap between the first check (line 177) and booking creation (line 205) allows a concurrent user to book the same room after the initial availability check passes (TOCTOU race condition). While BookingRepository.create() re-checks availability at line 106 inside a transaction, the separate callInTransaction() call for getEmptyRooms() may not provide sufficient isolation from concurrent bookings made by other threads.
Remove the redundant outer if at line 177, and refactor BookingRepository.create() to perform room selection and booking atomically within a single transaction (e.g., using JPA locking or a single unified query that both selects and reserves the room).
| String firstName = scanner.nextLine().trim().toLowerCase(); | ||
| System.out.print("Last name: "); | ||
| String lastName = scanner.nextLine().trim().toLowerCase(); |
There was a problem hiding this comment.
Guest names are converted to lowercase.
Converting first and last names to lowercase is unusual and may not match user expectations (e.g., "John Smith" becomes "john smith"). Consider preserving the original casing or using proper case formatting.
🤖 Prompt for AI Agents
In @src/main/java/org/example/Menu.java around lines 188 - 190, The code
currently forces guest names to lowercase by calling .toLowerCase() on scanner
input; remove the .toLowerCase() calls so firstName and lastName are assigned
with scanner.nextLine().trim() to preserve user casing, or alternatively apply a
proper capitalization routine (e.g., title-case each word) before storing;
update the two assignments for firstName and lastName (the lines currently using
.toLowerCase()) accordingly and ensure any downstream usage expects the
preserved/formatted casing.
…e of faulty business logic Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/main/java/org/example/BookingService.java:
- Around line 48-49: Update the user-facing error string in BookingService where
the start/end date check occurs: replace the current System.out.println("Start
date must from today and before end date.") with a grammatically correct message
such as "Start date must be today or later and before end date." so the output
tied to the startDate/isAfter(endDate) || startDate.isBefore(LocalDate.now())
condition reads clearly.
In @src/main/java/org/example/Menu.java:
- Around line 138-164: The loop can NPE because totalPrice is null when the
catch does a continue; initialize totalPrice to a non-positive BigDecimal (e.g.,
BigDecimal.ZERO or BigDecimal.valueOf(-1)) at declaration in createBooking so
Objects.requireNonNull(totalPrice).signum() is safe, or alternatively set
totalPrice = BigDecimal.ZERO inside the catch before continue; adjust the
totalPrice initialization and/or catch block so totalPrice is never null before
the while condition that checks totalPrice.signum(), while keeping the existing
calls to bookingRepo.getEmptyRooms(...) and
bookingService.calculateTotalPrice(...).
🧹 Nitpick comments (2)
src/main/java/org/example/Menu.java (2)
186-195: Names are lowercased, losing proper casing.Converting first and last names to lowercase (lines 188, 190) will store names as "john" instead of "John". This is unusual for proper nouns and may not match user expectations when displayed.
Consider preserving casing or applying proper title case
System.out.print("First name: "); - String firstName = scanner.nextLine().trim().toLowerCase(); + String firstName = scanner.nextLine().trim(); System.out.print("Last name: "); - String lastName = scanner.nextLine().trim().toLowerCase(); + String lastName = scanner.nextLine().trim();
177-208: Room availability may change between price confirmation and booking creation.The second
getEmptyRoomscheck on line 177 is redundant after the earlier check on line 158. More critically, there's a potential time-of-check to time-of-use (TOCTOU) issue where rooms could be booked between the user confirming the price (line 169) and the actual booking creation (line 205).Consider handling this at the repository level with proper transaction isolation, or at minimum, handle the case where
bookingRepo.createfails due to no available rooms.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/example/BookingService.javasrc/main/java/org/example/Menu.java
🔇 Additional comments (10)
src/main/java/org/example/BookingService.java (2)
10-18: LGTM!Constructor and field initialization are properly structured. Dependency injection of
BookingRepositoryis appropriate for the validation requirements.
27-38: LGTM!The price calculation logic is correct. The defensive
RuntimeExceptionon lines 33-34 is appropriate as a fail-fast mechanism since a non-positive price should be impossible after successful validation.src/main/java/org/example/Menu.java (8)
12-25: LGTM!Clean dependency injection setup with final fields. The class structure is well-organized for CLI menu handling.
27-58: LGTM!The main menu loop is well-structured with proper exception handling. Using string-based switch for menu choices is appropriate for CLI input.
61-79: LGTM!Proper empty-check before iterating bookings with clear formatting.
81-113: LGTM!Good validation sequence: empty check → guest existence → bookings retrieval. Clean separation of concerns.
115-136: LGTM!Properly validates booking ID before querying and handles the empty result case.
213-226: LGTM!Clean implementation with proper validation and error handling for the remove operation.
228-243: LGTM!Proper validation helper that checks both empty string and numeric format. The naming convention
isInvalidBookingIdreturningtruefor invalid cases is clear.
245-251: LGTM!Simple validation helper for guest names.
Fix possible NullPointerException in createBooking() Co-authored-by: Elias Lennheimer Lennheimer.elias@gmail.com Co-authored-by: Martin Karlsson Martin.Karlsson@iths.se Co-authored-by: Rickard Ankar rickard.ankar@iths.se
1 participant
Move CLI-menu logic to separate class Menu.java and add CLI menu structure for booking system
Summary by CodeRabbit
Refactor
New Features
Bug Fixes
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.