在线亚洲AV日韩AV综合AV,国产订精品电影,久久国产精品蜜

      <thead id="u9ncv"><del id="u9ncv"><rp id="u9ncv"></rp></del></thead>

        <thead id="u9ncv"></thead>

        <thead id="u9ncv"><s id="u9ncv"></s></thead>

          <strike id="u9ncv"></strike>
          <table id="u9ncv"><form id="u9ncv"></form></table>
        1. <strike id="u9ncv"></strike>
        2. 使用幫助 | 聯系電話:400-880-0256 0769-23037585 21686281

          Rails遺留程序中最常犯的錯誤(上)

          作者:admin 發表于:2014-07-24 點擊:1087  保護視力色:

          近來我一直在對幾個遺留項目作維護。

          眾所周知,處理遺留項目多數時間都感覺糟透了,因為那些代碼通常都丑陋不堪而且晦澀難懂。

          我決定做一個列表,記錄下那些公認的不良實踐,或者是我認為不太好的實踐,以及如何改良代碼來避免這些不良實踐。

          問題一覽

          • 在模型層以外使用查詢方法
          • 在視圖層使用業務邏輯
          • 使用無意義的方法名和變量名
          • 條件判斷時使用unless或者否定的表達式
          • 沒有遵循“命令,不要去詢問”原則
          • 使用復雜的條件
          • 在模型的實例方法里,本來不需要的時候使用了“self.”
          • 使用條件表達式并且返回了條件
          • 在視圖層使用行內樣式
          • 在視圖層使用JavaScript
          • 調用方法時把另一個方法的調用作為參數
          • 沒有使用類來隔離Rake Tasks
          • 沒有遵循Sandi Metz的規則

          在模型層以外使用查詢方法

          不好的

          # app/controllers/users_controller.rb
          class UsersController < ApplicationController
          
            def index
              @users = User.where(active: true).order(:last_login_at)
            end
          
          end
          

          這段代碼不可重用而且難于測試。如果你在別的地方也想查找全部用戶并進行排序,就會出現冗余代碼。

          好的

          比起在控制器里使用查詢方法,我們的做法是在模型層中使用scope把它們獨立出來,就如下例所示。這樣做既可以使代碼能夠復用,又便于代碼閱讀和測試。

          # app/models/user.rb
          class User < ActiveRecord::Base
          
            scope :active, -> { where(active: true) }
            scope :by_last_login_at, -> { order(:lasst_login_at) }
          
          end
          
          # app/controllers/users_controller.rb
          class UsersController < ApplicationController
          
            def index
              @users = User.active.by_last_login_at
            end
          
          end
          

          每當你想用where、order、joins、includes、group、having或者其他查詢方法時,記得要把它們放在模型層里。

          在視圖層使用業務邏輯

          不好的

          <!-- app/views/posts/show.html.erb -->
          ...
          <h2>
            <%= "#{@comments.count} Comment#{@comments.count == 1 ? '' : 's'}" %>
          </h2>
          ...
          

          初看之下這小段代碼似乎沒什么問題,但是它會讓HTML變得有點難以閱讀,而且可以肯定的說一旦你在視圖層添加了邏輯代碼,那么日后你定會添加更多的邏輯到視圖。這段代碼還有一個問題,里面的邏輯無法復用,而且不能單獨測試。

          好的

          使用Rails的helper方法把業務邏輯隔離出來

          # app/helpers/comments_helper.rb
          module CommentsHelper
            def comments_count(comments)
              "#{comments.count} Comment#{comments.count == 1 ? '' : 's'}"
            end
          end
          
          <!-- app/views/posts/show.html.erb -->
          <h2>
            <%= comments_count(@comments) %>
          </h2>
          

          使用無意義的方法名和變量名

          不好的

          # app/models/topic.rb
          class Topic < ActiveRecord::Base
          
            def self.r_topics(questions)
              rt = []
          
              questions.each do |q|
                topics = q.topics
          
                topics.each do |t|
                  if t.enabled?
                    rt << t
                  end
                end
              end
          
              Topic.where(id: rt)
            end
          
          end
          

          這類遺留代碼最主要的問題在于:你需要花費大把時間來搞清楚這些代碼的用途。r_topics這個方法是做什么的,rt這個變量又是什么意思。其他的一些變量,比如在代碼塊里用到的那個,變量名的含義很模糊,這樣也使得它們的用途初看起來很難理解。

          好的

          對方法和變量命名時選那些能表達出其含義的名字。這樣更便于其他開發者理解你的代碼。

          # app/models/topic.rb
          class Topic < ActiveRecord::Base
          
            def self.related(questions)
              related_topics = []
          
              questions.each do |question|
                topics = question.topics
          
                topics.each do |topic|
                  if topic.enabled?
                    related_topics << topic
                  end
                end
              end
          
              Topic.where(id: related_topics)
            end
          
          end
          

          這樣改進的好處在于:

          • 第一次看到方法名時就會對方法返回值有個概念。一個與給定問題集合相關聯的主題的集合。
          • 現在你能夠了解related_topics表示一個數組,它里面存放了一個與給定問題集合相關聯的主題的集合。之前打代碼里rt表示什么非常含糊。
          • 使用topic代替之前的t,并用question替換掉q,使得你的代碼更便于閱讀,因為你不再需要腦補這些變量的用途?,F在這些代碼已然能夠自解釋一切。

          條件判斷時使用unless或者否定的表達式

          不好的

          # app/services/charge_user.rb
          class ChargeUser
          
            def self.perform(user, amount)
              return false unless user.enabled?
          
              PaymentGateway.charge(user.account_id, amount)
            end
          
          end
          

          這段代碼也許并不難理解,但是使用unless或者否定的條件表達式會稍微增加代碼的發復雜度,因為你必須對它要判斷的條件自行腦補。

          好的

          改用if或者肯定的條件表達式之后,上述代碼就會好懂得多。

          # app/models/user.rb
          class User < ActiveRecord::Base
          
            def disabled?
              !enabled?
            end
          
          end
          
          # app/services/charge_user.rb
          class ChargeUser
          
            def self.perform(user, amount)
              return false if user.disabled?
          
              PaymentGateway.charge(user.account_id, amount)
            end
          
          end
          

          不覺得這樣寫代碼更易讀了嗎?我更傾向于使用if而非unless,用肯定的表達式多過肯定的表達式。實在不行就添加個反意的方法,比如我們在User模型里加的那個。

          沒有遵循“命令,不要去詢問”原則

          不好的

          # app/models/user.rb
          class User < ActiveRecord::Base
          
            def enable!
              update(enabled: true)
            end
          
          end
          
          # app/controllers/users_controller.rb
          class UsersController < ApplicationController
          
            def enable
              user = User.find(params[:id])
          
              if user.disabled?
                user.enable!
                message = "User enabled"
              else
                message = "User already disabled"
              end
          
              redirect_to user_path(user), notice: message
            end
          
          end
          

          這里的問題是在不恰當的地方出現了控制邏輯。你先判斷了用戶是否是不可用,如果的確不可用,就啟用這個用戶。

          好的

          比較好的改辦法是把控制邏輯放到enable!方法里。

          # app/models/user.rb
          class User < ActiveRecord::Base
          
            def enable!
              if disabled?
                update(enabled: true)
              end
            end
          
          end
          
          # app/controllers/users_controller.rb
          class UsersController < ApplicationController
          
            def enable
              user = User.find(params[:id])
          
              if user.enable!
                message = "User enabled"
              else
                message = "User already disabled"
              end
          
              redirect_to user_path(user), notice: message
            end
          
          end
          

          現在控制器不用關心user需要滿足何種條件才會被啟用。相關的判斷由User模型和enable!方法來處理。

          使用復雜的條件

          不好的

          # app/controllers/posts_controller.rb
          class PostsController < ApplicationController
          
            def destroy
              post = Post.find(params[:id])
          
              if post.enabled? && (user.own_post?(post) || user.admin?)
                post.destroy
                message = "Post destroyed."
              else
                message = "You're not allow to destroy this post."
              end
          
              redirect_to posts_path, notice: message
            end
          
          end
          

          條件表達式弄的太過復雜了,實際上這里只想知道一件事:用戶可以刪掉post嗎?

          好的

          從上面的代碼我們可以了解到,一個用戶需要是post的所有者或者這個用戶是管理員,并且post本身也是可用的,才可以刪除這個post。最好的做法就是,把這些條件抽取成一個日后可以復用的方法。

          # app/models/user.rb
          class User < ActiveRecord::Base
          
            def can_destroy_post?(post)
              post.enabled? && (own_post?(post) || admin?)
            end
          
          end
          
          # app/controllers/posts_controller.rb
          class PostsController < ApplicationController
          
            def destroy
              post = Post.find(params[:id])
          
              if user.can_destroy_post?(post)
                post.destroy
                message = "Post destroyed."
              else
                message = "You're not allow to destroy this post."
              end
          
              redirect_to posts_path, notice: message
            end
          
          end
          

          每當條件表達式里出現了&&或者||,就應該把它們提取為方法,以備以后復用。

          Rails遺留程序中最常犯的錯誤(上),首發于博客 - 伯樂在線。

          在线亚洲AV日韩AV综合AV,国产订精品电影,久久国产精品蜜