Question

I've got a rather simple Grails controller action which binds the parameters to a domain instance and passes that to a service which handles the persistence.

def finishBooking() {
    Booking booking = new Booking(params)
    try {
        booking = bookingService.saveBooking(booking)
    } catch (ReservationException e) {
        log.error("Error saving booking", e)
        flash.message = "Couldn't save the reservation."
        render(view: "index", model: [booking: booking])
        return
    }
    if (booking.hasErrors()) {
        flash.message = "Reservation failed. Check the required fields."
        render(view: "index", model: [booking: booking])
    } else {
        [booking: booking]
    }
}

According to codenarc, the return statement in the catch block is a bad practice. How else would you implement the error handling?

Was it helpful?

Solution

You don't do anything important in you catch block. What will codenarc will say on this(move return to try block):

def finishBooking() {
    Booking booking = new Booking(params)
    try {
        booking = bookingService.saveBooking(booking)
        if (!booking.hasErrors()) {
            return [booking: booking]
        } else {
            flash.message = "Reservation failed. Check the required fields."
        }
    } catch (ReservationException e) {
        log.error("Error saving booking", e)
        flash.message = "Couldn't save the reservation."
    }
    render(view: "index", model: [booking: booking])
}

P.S. Thanks for the link. Never heard about codenarc.

OTHER TIPS

+1 @Mr. Cat. Something like.

def finishBooking() {
    Booking booking = new Booking(params)
    try {
        booking = bookingService.saveBooking(booking)
        if (booking.hasErrors()) {
            flash.message = "Reservation failed. Check the required fields."
            render(view: "index", model: [booking: booking])
        }
    } catch (ReservationException e) {
        log.error("Error saving booking", e)
        flash.message = "Couldn't save the reservation."
        render(view: "index", model: [booking: booking])
    }

    [booking: booking]
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top