请提供 Groovy/Grails 代码清理建议！答案

【问题标题】：Groovy/Grails code cleanup suggestions, please!请提供 Groovy/Grails 代码清理建议！
【发布时间】：2010-12-12 03:50:46
【问题描述】：

我是 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'
            }
        }
    }
}

【问题讨论】：

标签： grails groovy coding-style

【解决方案1】：

我实际上会开始使用Command Objects。

试试这个：

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”

【讨论】：

Command 对象会自动执行获取 Car/Wheel 所需的 GORM 内容吗？我不认为这是真的，在他的代码中的某个地方他仍然必须有 Wheel.findById(cmd.wheel.id) 和 Car.findById(cmd.car.id)。
这是一个工作示例：github.com/ColinHarrington/CommandObjectExample (grails 1.3.5)
知道了，所以“if(!cmd.hasErrors())”会更合适。我确实认为 if(!cmd.hasErrors()) 或 if(cmd.validate()) 看起来比 if(cmd.wheel && cmd.car) 更干净，作为验证 changeWheel 输入的一种方式。
@Alison 它基本上对命令对象grails.org/doc/latest/guide/…进行数据绑定
@Alison 你也可以阅读 bindData() grails.org/doc/latest/ref/Controllers/bindData.html

【解决方案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 的语法糖做太多事情，但也许一些 groovy 大师可以提供更好的建议。

【讨论】：

嘿，谢谢你的其余部分 :-) 去嵌套看起来不错；我也希望 Groovy 大师建议我用一个神奇的闭包替换整个东西。顺便说一句，在您的回答中您忽略了 save() 失败。
@alison，您会注意到在控制器中定义闭包，在服务中定义方法。所以你已经有一个闭包了。
是的。那么在控制器中创建方法是不好的做法吗？例如私有方法getCarFromParams()
@alison，不，实用方法很好。被调用的动作应该是闭包。
当您可以使用 Groovy Truth 时，为什么要进行空检查？即： if (car && wheel) { ... }

【解决方案3】：

您可以做一些事情，例如将一些代码移动到服务或命令对象。但在不过多改变结构的情况下，我（主观上）认为以下内容会使代码更易于阅读：

使用点符号代替数组索引来引用参数值（params.wheelId 代替 params['wheelId']）
我会反转 if 以减少嵌套，我认为这可以更清楚地说明异常是什么。

例如：

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

现在，如果您不介意更改结构并添加更多代码行...

更换轮子的行为可能在不止一个控制器操作中经常发生。在这种情况下，我建议将 GORM/数据库逻辑放在服务类中。然后您的控制器只需验证它是否具有正确的 params 输入并将其传递给服务以进行实际的轮胎更换。 Service 方法可以是事务性的，在安装新轮胎之前可能必须卸下旧轮胎的情况下，您会需要这种方法。

在服务中，我会针对特殊情况抛出异常，例如找不到车轮、找不到汽车或更换轮胎时出错。然后您的控制器可以捕获这些并使用正确的 HTTP 状态代码进行响应。

【讨论】：