最近这段时间一直在读大家写的代码，发现有时候不好的代码有些是因为不了解程序运行的本质，有些代码是因为没有用心
这次我就分享一些代码中的简单的错误
-
数据库查询多次，改变写法可以减少数据库查询次数，降低数据库压力
if Employee.find_by_id(12345) and Employee.find_by_id(12345).username=='bad.code'
如上这段代码，很明显会查询两次数据库，但是完全是没有必要的
发生错误的原因我猜有两种可能
1.不了解rails查询的本质，不知道上面会查询两次(这种可能比较小)
2.没有用心去想程序应该怎么写更好，实现功能就好users = User.all(:conditions=>['status=:status',{:status=>1}]) users.each do |user| company = Company.find_by_id(user.company_id) .....其他操作 end
如上这段代码，company查询了多次，其实我们可以在each外面先一次查询出来，这样就不需要多次查询数据库了，形成如下代码
users = User.all(:conditions=>['status=:status',{:status=>1}])
companies = Company.all(:conditions=>['id in (?)',users.map(&:company_id)])
users.each do |user|
company = companies.find{|com|com.id==user.company_id}
.....其他操作
end
上面的代码和数据库的交互就比较友好了，不过仔细观察如上代码，其实User和Company是存在关联关系(relation)的，一般我们已经定义好了他们之间的关系，所以我们的代码其实应该是这样的
users = User.all(:conditions=>['status=:status',{:status=>1}],:include=>[:company])
users.each do |user|
company = user.company
.....其他操作
end
- 代码嵌套层数比较多
if user.present?
if company.present?
return "#{company.name}-#{user.name}"
else
return '公司不存在'
end
else
return '用户不存在'
end
如上代码，符合人的正常思维逻辑并没有问题，但是是不是有点长呢？ 其实我们可以反过来先判断不存在的情况，代码可以简化为
return '用户不存在' if user.blank?
return '公司不存在' if company.blank?
"#{company.name}-#{user.name}"
代码是不是简洁多了呢？我个人把这种情况叫做“尽早返回原则”，就是如果满足某一个条件就可以忽略其他条件的话，那么我们可以先判断这个条件以简化代码。
