How to avoid returning null from a catch block?

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?

You would not need the `return` in catch. You are rendering a view already. — dmahapatro, May 23 '13 at 15:27
How about place all code to try block and remove return from catch? — Mr. Cat, May 23 '13 at 15:27
@Mr.Cat Did not mean to step on your toes... You have all the credit. :) — dmahapatro, May 23 '13 at 15:40

score 3 · Accepted Answer · answered May 23 '13 at 15:43

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.

Infact we have a plugin for [codenarc](http://grails.org/plugin/codenarc). It has been awesome yet. :) — dmahapatro, May 23 '13 at 15:46

score 2 · Answer 2 · answered May 23 '13 at 15:29

+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]
}

How to avoid returning null from a catch block?

2 Answers2