师傅领错门，害了你个人 - ruby/rails新手避免入错门

最近看到一位Ruby On Rails新手写的代码，简直是惨不忍睹，问了一下他竟然是用李刚写的那本<<Ruby on Rails敏捷开发最佳实践>>作为入门材料的，偶真是faint，啥也不多说了，为了让更多的Ruby On Rails的新手避免走弯路，偶觉得很有必要来评论一下这本书，以china pub上下载的第15章样章为例子：
http://www.china-pub.com/39348

这章是讲一个注册用户通过邮件注册激活的例子，偶们来看看所谓的最佳实践实际是如何成为绝佳反面教材：
1. 创建一个用户表
这里使用的是sql，而不是Rails最佳实践推荐的migration

2. 用户表中直接用明码保存用户的密码
请各位作实际开发新手注意：任何一个应用都不应该犯这样愚蠢的错误

3. 标示用户是否激活的字段名叫is_actived
这不符合rails的最佳实践写法，对于boolean类型的数据，应该省略前面的is_，ActiveRecord会自动加个?号，映射成actived?

4. User模型的校验功能用errors.add这样手工的方法
最佳实践是使用Validation DSL，除非ActiveRecord提供的DSL不能满足你的特殊校验需求，否则毫无必要自己手工处理
就算你有很特殊的校验需求，在这段代码里面也应该使用errors.add_to_base，而不是用errors.add_base，跟一个空白的字符串

5. 设置用户的默认激活为false：@user.is_activated = false
Rails的理念是COC和DRY，这种初始默认值的设定应该在创建数据库定义的时候指定，而且默认的boolean都是false，这里的赋值完全是多此一举

让我们先休息和回顾一下，上面提到的5个绝佳反面教材竟然是出现在一个章节中的一个小段：15.4.1基本注册功能，错误之密集真是令人咋舌。
如果你和偶一样，还能忍受下去，恭喜你，你具备了给烂代码作Code Review的基本要求：耐力。

偶们来继续往下看:
6. 看看发送邮件的代码

@sent_on = Time.now
@headers = {}

@sent_on指定的是邮件头信息中的Date，默认就是使用当前时间，这里的赋值也是多此一举
@headers指定的是邮件头信息的hash，这里也是也是多此一举
结合之前的问题5，李刚老师您这是用无用代码来充书本厚度么？


7. if user != nil && user.is_activated == false
user.is_activated == false ??? 这叫啥代码阿？ 偶的天哪，李刚老师可能是为了保持风格统一，后面果然还有 user.is_activated == true ，这是偶看到搞笑的代码了...

8. pro_activate Action的代码写得及其冗长
综合前面的错误，偶想李刚可能压根不懂一些ruby和rails的惯用法，偶好为人师一下，符合ruby风格的代码应该是这样：

def pro_activate
  user = User.find_by_name_and_active_code(params[:name], params[:active_code])
  if user 
    if user.actived?
      flash[:notice] = "您的账户已经处于激活状态,请勿重复激活!"		
    else
      user.update_attribute(:activated , true)
      flash[:notice] = "恭喜您,您已经成功激活了您的账户!"
    end
  else
    flash[:notice] = "激活失败!"
  end
end

而且在实际开发中，作为最佳实践，最后一个else判断完全是不必要的，大家可以想想什么情况下会出现跑到最后一个else？嗯，只有在恶意构造url攻击的情况下，这样我们完全可以改成：
User.find_by_name_and_active_code_and_active(params[:name], params[:active_code], false)
在之后的pro_login action里面的代码就不多说了，和前面一样，也有user != nil && user.is_activated == false这样搞笑的代码出现。

看完了这个短短5页的样章中有那么多的错误，偶明白了为什么那个新手写的代码是如此的惨不忍睹，虽然俗话说，师傅领进门，修行在个人，但是也要晓得：师傅领错门，害了你个人
最后推荐了另外几本Ruby和Rails的书给他，让他好好改造去了...

评论

20 楼 lonelyblue 2008-11-11  

要是再打给肉饼，肉饼多冤哪。。。。。。

19 楼 www_zr 2008-11-11  

抛出异常的爱 写道

上回给肉饼打电话的是哪个?

这次还得打

18 楼 gigix 2008-11-11  

下一站，火星 写道

范三山 写道

下一站，火星 写道

分析的不错!


《Web开发大全&mdash;&mdash;Ruby on Rails版》（http://product.dangdang.com/product.aspx?product_id=20319929）这本书貌似不错，驴子以为如何？

那是兔子的说......


这本书作者写着  &ldquo;编委会&rdquo; 著 是什么意思？

这本书是gigix做的序 ， 5个作者起码有4个在JE，大家伙基本都认识，看目录的话，感觉编排的内容很丰富

这本书，我的观点是，至少这几个人是靠谱的，是会写程序的，那么写出书来就算不好看至少不害人（当然实际上我个人觉得是还算好看的）
连程序都不会写的人还写书教别人怎么写程序，这个世界我真的看不懂

17 楼 庄表伟 2008-11-11  

抛出异常的爱 写道

上回给肉饼打电话的是哪个?

是李刚他们培训学校的校长——田江东

参见：http://www.iteye.com/topic/223451
http://www.iteye.com/topic/223273

16 楼 下一站，火星 2008-11-11  

<div class='quote_title'>抛出异常的爱 写道</div>
<div class='quote_div'>上回给肉饼打电话的是哪个?</div>
<p> </p>
<p>见楼下</p>
<p> </p>
<p>PS：到处找你：<a href='http://mars.group.iteye.com/group/topic/7942'>http://mars.group.iteye.com/group/topic/7942</a></p>

15 楼 下一站，火星 2008-11-11  

<div class='quote_title'>范三山 写道</div>
<div class='quote_div'>
<div class='quote_title'>下一站，火星 写道</div>
<div class='quote_div'>
<p>分析的不错!</p>
<p> </p>
<p>《Web开发大全——Ruby on Rails版》（<a href='http://product.dangdang.com/product.aspx?product_id=20319929'>http://product.dangdang.com/product.aspx?product_id=20319929</a>）这本书貌似不错，驴子以为如何？</p>
</div>
<p>那是兔子的说......</p>
<p> </p>
<p>这本书作者写着  “编委会” 著 是什么意思？</p>
</div>
<p><br/>这本书是gigix做的序 ， 5个作者起码有4个在JE，大家伙基本都认识，看目录的话，感觉编排的内容很丰富</p>
<p> </p>
<p> </p>

14 楼 抛出异常的爱 2008-11-11  

上回给肉饼打电话的是哪个?

13 楼 范三山 2008-11-11  

<div class='quote_title'>下一站，火星 写道</div>
<div class='quote_div'>
<p>分析的不错!</p>
<p> </p>
<p>《Web开发大全——Ruby on Rails版》（<a href='http://product.dangdang.com/product.aspx?product_id=20319929'>http://product.dangdang.com/product.aspx?product_id=20319929</a>）这本书貌似不错，驴子以为如何？</p>
</div>
<p>那是兔子的说......</p>
<p> </p>
<p>这本书作者写着  “编委会”  著 是什么意思？</p>

12 楼 庄表伟 2008-11-11  

这种人的名字，应该牢牢记住！

以后只要看到，就避而远之！

11 楼 ozzzzzz 2008-11-11  

电话，电话，电话

10 楼 upheart 2008-11-11  

if user != nil && user.is_activated == false
佩服，就是java代码也不能这么写啊
李刚好像什么都精通奥，在书店好像还看到他的一本flex的书

9 楼 daquan198163 2008-11-11  

<div class='quote_title'>下一站，火星 写道</div>
<div class='quote_div'>
<p>分析的不错!</p>
<p> </p>
<p>《Web开发大全——Ruby on Rails版》（<a href='http://product.dangdang.com/product.aspx?product_id=20319929'>http://product.dangdang.com/product.aspx?product_id=20319929</a>）这本书貌似不错，<span style='color: #ff0000;'>驴子</span>以为如何？</p>
</div>
<p> </p>

8 楼 下一站，火星 2008-11-11  

<p>分析的不错!</p>
<p> </p>
<p>《Web开发大全——Ruby on Rails版》（<a href='http://product.dangdang.com/product.aspx?product_id=20319929'>http://product.dangdang.com/product.aspx?product_id=20319929</a>）这本书貌似不错，驴子以为如何？</p>

7 楼 上一站，火星 2008-11-11  

李刚曰：我告你诽谤

6 楼 caryl 2008-11-11  

小心李刚告你，哈

5 楼 fujohnwang 2008-11-11  

呵呵，有些问题可能是你说的那样，有些问题也可能是作者想要简化讲解的内容，很难说的 还有可能就是作者其实也只是入门...god knows

4 楼 Ari9x 2008-11-11  

<div class='quote_title'>Readonly 写道</div>
<div class='quote_div'>
<div class='quote_title'>Ari9x 写道</div>
<div class='quote_div'><br/>
<div class='quote_title'>Readonly 写道</div>
<div class='quote_div'>最后推荐了另外几本Ruby和Rails的书给他，让他好好改造去了...</div>
<br/><br/>透漏下呗 <br/><br/></div>
<br/>为了避免偶给另外的书作托的嫌疑，偶只给读书建议： <br/>1. 读原版，选amazon上排前面的 <br/>2. 英文吃力的，读1的翻译作品，同时也要看译者以往作品的口碑 <br/>3. 国内原创的，首先要看作者是否有实际经验，同时注意广告，广告打的越狠的，往往不怎么滴</div>
<p> </p>
<p>短信PM我 ,谢谢</p>
<p>偶会保密滴~~</p>
<p> </p>

3 楼 Readonly 2008-11-11  

Ari9x 写道

Readonly 写道

最后推荐了另外几本Ruby和Rails的书给他，让他好好改造去了...

透漏下呗

为了避免偶给另外的书作托的嫌疑，偶只给读书建议：
1. 读原版，选amazon上排前面的
2. 英文吃力的，读1的翻译作品，同时也要看译者以往作品的口碑
3. 国内原创的，首先要看作者是否有实际经验，同时注意广告，广告打的越狠的，往往不怎么滴

2 楼 阳光晒晒 2008-11-11  

只要挣钱什么书都敢写啊.

1 楼 Ari9x 2008-11-11  

<div class='quote_title'>Readonly 写道</div>
<div class='quote_div'>最后推荐了<span style='color: #ff0000;'>另外几本</span>Ruby和Rails的书给他，让他好好改造去了...</div>
<p> </p>
<p> 透漏下呗</p>