写代码那点事（二）分享一些代码

最近这段时间一直在读大家写的代码，发现有时候不好的代码有些是因为不了解程序运行的本质，有些代码是因为没有用心
这次我就分享一些代码中的简单的错误

• 数据库查询多次，改变写法可以减少数据库查询次数，降低数据库压力
  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}"
  代码是不是简洁多了呢？我个人把这种情况叫做“尽早返回原则”，就是如果满足某一个条件就可以忽略其他条件的话，那么我们可以先判断这个条件以简化代码。