Советы по очистке кода Groovy / Grails, пожалуйста!

Question

Я новичок в Groovy & Grails, и у меня есть ощущение, что вещи не должны быть такими уродливыми ... так как я могу сделать этот код лучше?

Это класс контроллера Grails, за исключением некоторых неинтересных битов. Постарайтесь не зацикливаться на том, что у моего Car есть только один Wheel - с этим я справлюсь позже: -)

changeWheel - это действие Ajax.

class MyController {
    ...
    def changeWheel = {
        if(params['wheelId']) {
            def newWheel = Wheel.findById(params['wheelId'])
            if(newWheel) {
                def car = Car.findById(params['carId'])
                car?.setWheel(newWheel)
                if(car?.save()) render 'OK'
            }
        }
    }
}

Answer 1

Я бы на самом деле начал использовать Объекты команд .

Попробуйте это:

class MyController {
    def index = {
    }
    def changeWheel = { CarWheelCommand cmd ->
        if(cmd.wheel && cmd.car) {
            Car car = cmd.car
            car.wheel = cmd.wheel
            render car.save() ? 'OK' : 'ERROR'
        } else {
            render "Please enter a valid Car and wheel id to change"
        }
    }
}
class CarWheelCommand {
    Car car
    Wheel wheel
}

и затем, по вашему мнению, используйте 'car.id' и 'wheel.id' вместо 'carId' и 'wheelId'

Answer 2

1) тянуть

params['wheelId'] 

и

params['carId']

в свои собственные определения

2) множественные вложенные if никогда не оптимальны. Вы можете избавиться от самого внешнего, используя метод validateParams и предоставляя какой-то ответ, если wheelId и carId не установлены. Или просто сделай

if (carId == null || wheelId == null) {
    // params invalid
}

3) Предполагая, что все в порядке, вы можете просто сделать

def newWheel = Wheel.findById...
def car = Car.findById...
if (car != null && newWheel != null) {
    car.setWheel(newWheel)
    car.save()
    render 'OK'
} else {
   // either wheel or car is null

}

это избавляет от более вложенных структур ...

4) наконец, чтобы сделать код самодокументированным, вы можете сделать такие вещи, как назначить условные тесты переменным с соответствующими именами. Так что-то вроде

def carAndWheelOk = car != null && newWheel != null
if (carAndWheelOk) {
   // do the save
} else {
   // car or wheel not ok
}

это может быть излишним для двух тестов, но вы заботитесь только об одном колесе здесь. Если вы имели дело со всеми 4 колесами, этот тип вещей повышает удобочитаемость и удобство обслуживания.

Обратите внимание, что этот совет работает на любом языке. Я не думаю, что вы можете сделать слишком много с синтаксическим сахаром Groovy, но, возможно, некоторые отличные гуру могут предложить лучший совет.

Answer 3

Есть несколько вещей, которые вы можете сделать, например, переместить некоторый код в объект службы или команды.Но, не изменяя структуру слишком сильно, я (субъективно) думаю, что следующее облегчит чтение кода:

1. использование точечной нотации вместо индексации массива для ссылки на значения параметров (params.wheelId)вместо params ['wheelId'])

2. Я бы перевернул оператор if, чтобы уменьшить вложенность, я думаю, это делает более понятным, что такое исключения.

Например:

if(!params.wheelId) {
    sendError(400, "wheelId is required")
    return
}
....
....
if(!newWheel) {
    sendError(404, "wheel ${params.wheelId} was not found.")
    return
}

Теперь, если вы не возражаете, измените структуру и добавьте больше строк кода ...

Смена колеса может бытьобычное явление для более чем одного действия контроллера.В этом случае я бы рекомендовал поместить логику GORM / базы данных в класс Service.Тогда ваш контроллер должен только убедиться, что он имеет правильные входные данные params и передать их в службу для фактической замены шин.Сервисный метод может быть транзакционным, что вам может понадобиться в случае, если вам, возможно, придется демонтировать старую шину перед установкой новой.

В Сервисе я бы выбрасывал исключения для исключительных случаев, например, когда колесо не найдено, автомобиль не найден или если произошла ошибка при смене шины.Затем ваш контроллер может их перехватить и ответить правильными кодами состояния HTTP.