在模型的例項方法裡，本來不需要的時候使用了“self.”
不好的
|
1 2 3 4 5 6 7 8 9 |
; html-script: false ] # app/models/user.rb class User < ActiveRecord::Base def full_name "#{self.first_name} #{self.last_name}" end end |
這段程式碼並不複雜但是裡面並不需要使用“self.”。把“self.”去掉會使程式碼更簡潔且不影響可用性。
好的
在模型裡，只有在例項方法裡需要賦值時，才會用到“self.”，否則通篇的“self.”只會徒增程式碼複雜度。
|
1 2 3 4 5 6 7 8 9 |
; html-script: false ] # app/models/user.rb class User < ActiveRecord::Base def full_name "#{first_name} #{last_name}" end end |
使用條件表示式並且返回了條件
不好的
|
1 2 3 4 5 6 7 8 9 10 11 12 13 |
; html-script: false ] # app/models/user.rb class User < ActiveRecord::Base def full_name if name name else "No name" end end end |
或者
|
1 2 3 4 5 6 7 8 9 |
; html-script: false ] # app/models/user.rb class User < ActiveRecord::Base def full_name name ? name : "No name" end end |
這段程式碼的問題在於：在不需要的地方新增了控制語句。
好的
有種更簡便的處理方式也能達到同樣效果
|
1 2 3 4 5 6 7 8 9 |
; html-script: false ] # app/models/user.rb class User < ActiveRecord::Base def full_name name || "No name" end end |
簡單來說這段程式碼會在name不為false或nil時將其返回，否則返回”No name”.
使用得當的話，||和&&這些操作符會對提升你的程式碼品質提供巨大助力。
在檢視層使用行內樣式
不好的
|
1 2 3 4 5 6 7 |
; html-script: false ] <!-- app/views/projects/show.html.erb --> ... <h3 style="font-size:20px;letter-spacing:normal;color:#95d60a;line-height:100%;margin:0;font-family:'Proxima Nova';"> SECRET PROJECT </h3> ... |
這裡我們只列出一個標籤，所有的樣式都寫在了標籤裡。現在，請設想一下，如果所有的標籤都接收行內樣式。這會把你的HTML變得和其難度，除此之外，每當你需要引入另一個同樣的h3元素時，將不得不把同樣程式碼照搬一邊，造成冗餘。
好的
|
1 2 3 4 5 6 7 8 9 10 |
; html-script: false ] // app/assets/stylesheets/application.css .project-title { font-size: 20px; letter-spacing: normal; color: #95d60a; line-height: 100%; margin: 0; font-family:'Proxima Nova'; } |
|
1 2 3 4 5 6 7 |
; html-script: false ] <!-- app/views/projects/show.html.erb --> ... <h3 class="project-title"> SECRET PROJECT </h3> ... |
現在我麼可以複用樣式了，並且HTML的可讀性也有所提高。
注意：這只是個簡單的範例，實際應用時你應該把CSS拆分成多個小檔案，並通過application.css來載入這些檔案。另外只有在email模板裡，才會用到行內樣式。
在檢視層使用JavaScript
不好的
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 |
; html-script: false ] <!-- app/views/questions/show.html.erb --> ... <textarea rows="4" cols="50" class='wysihtml5'> Insert your question details here. </textarea> ... <script> $(document).ready(function(){ $('textarea.wysihtml5').wysihtml5({ "font-styles": true, //Font styling, e.g. h1, h2, etc. Default true. "emphasis": true, //Italics, bold, etc. Default true. "lists": true, //(Un)ordered lists, e.g. Bullets, Numbers. Default true. "html": false, //Button which allows you to edit the generated HTML. Default false. "link": true, //Button to insert a link. Default true. "image": true, //Button to insert an image. Default true. "color": true //Button to change color of font. Default true. }); }); <script> |
這裡的邏輯和特定頁面耦合在一起，導致程式碼不可複用。
好的
Rails裡面有專門用於組織和存放javascript程式碼的地方：“app/assets/javascripts/”。
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 |
; html-script: false ] // app/assets/javascripts/application.js ... $(document).ready(function(){ $('textarea.wysihtml5').wysihtml5({ "font-styles": true, //Font styling, e.g. h1, h2, etc. Default true. "emphasis": true, //Italics, bold, etc. Default true. "lists": true, //(Un)ordered lists, e.g. Bullets, Numbers. Default true. "html": false, //Button which allows you to edit the generated HTML. Default false. "link": true, //Button to insert a link. Default true. "image": true, //Button to insert an image. Default true. "color": true //Button to change color of font. Default true. }); }); ... |
|
1 2 3 4 5 6 7 |
; html-script: false ] <!-- app/views/questions/show.html.erb --> ... <textarea rows="4" cols="50" class='wysihtml5'> Insert your question details here. </textarea> ... |
現在我們可以在view層任何地方用這段程式碼了。只需要頁面上有一個帶有wysihtml5這個class的textarea，剛才的那段js就會被執行。
注意：這只是個簡單的範例，實際應用時需要考慮是否需要把你的JavaScript拆分成若干小的檔案，並通過application.js來載入這些檔案。另外，如果你使用的是CoffeeScript而非JavaScript，請堅持不要把CoffeeScript與普通JavaScript在一起混寫。
呼叫方法時把另一個方法的呼叫作為引數
不好的
|
1 2 3 4 5 6 7 8 9 |
; html-script: false ] # app/services/find_or_create_topic.rb class FindOrCreateTopic ... def self.perform(user, name) find(user, sluggify(name)) || create(user, name) end ... end |
這段程式碼裡呼叫了find方法並傳入了2個引數，首引數為user，第二個引數則是直接呼叫了sluggify這個方法並把name作為引數傳給sluggify。你也許會有疑問，這麼寫有什麼問題嗎？我明明完全能夠看懂這段程式碼呀。是的，程式碼也許不難理解，但是每次到這裡你都需要自己做一點腦筋轉換，而這正是我一直極力想要避免的。
好的
避免需要腦筋轉換的一個比較有效的辦法就是：使用有意義的變數名。
|
1 2 3 4 5 6 7 8 9 10 |
; html-script: false ] # app/services/find_or_create_topic.rb class FindOrCreateTopic ... def self.perform(user, name) slug = sluggify(name) find(user, slug) || create(user, name) end ... end |
這樣做可以避免腦筋轉換。換用含義明確的變數名之後，每當你再呼叫find方法，就會知道find接受一個user和一個slug做引數。
沒有使用類來隔離Rake Tasks
不好的
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 |
; html-script: false ] # lib/tasks/recalculate_badges_for_users.rake namespace :users do desc "Recalculates Badges for Users" task recalculate_badges: :environment do user_count = User.count User.find_each do |user| puts "#{index}/#{user_count} Recalculating Badges for: #{user.email}" if user.questions_with_negative_votes >= 1 || user.answers_with_negative_votes >= 1 user.grant_badge('Critic') end user.answers.find_each do |answer| question = answer.question next unless answer && question days = answer.created_at - question.created_at days_count = (days / 1.day).round if (days_count >= 60) && (question.vote_count >= 5) grant_badge('Necromancer') && return end end end end end |
這個rake task有問題多多。最主要的問題是，這個rake太長了而且不好測試。程式碼寫的初一看也很難理解。你只好相信這個task的確是為使用者重新計算獎章系統的，因為它上面描述就這麼寫的。
好的
解決這個問題最好的辦法就是，把業務邏輯挪到一個特定的類裡面。所以，讓我們新建個類來搞定它吧。
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 |
; html-script: false ] # app/services/recalculate_badge.rb class RecalculateBadge attr_reader :user def initialize(user) @user = user end def grant_citric if grant_citric? user.grant_badge('Critic') end end def grant_necromancer user.answers.find_each do |answer| question = answer.question next unless answer && question if grant_necromancer?(answer, question) grant_badge('Necromancer') && return end end end protected def grant_citric? (user.questions_with_negative_votes >= 1) || (user.answers_with_negative_votes >= 1) end def days_count(answer, question) days = answer.created_at - question.created_at (days / 1.day).round end def grant_necromancer?(answer, question) (days_count(answer,question) >= 60) && (question.vote_count >= 5) end end |
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 |
; html-script: false ] # lib/tasks/recalculate_badges_for_users.rake namespace :users do desc "Recalculates Badges for Users" task recalculate_badges: :environment do user_count = User.count User.find_each do |user| puts "#{index}/#{user_count} Recalculating Badges for: #{user.email}" task = RecalculateBadge.new(user) task.grant_citric task.grant_necromancer end end end |
如你所見，現在這個rake task可讀性要好的多，而且還可以單獨測試每一種批准徽章的方法。除此以外我麼也可以在有需要時複用這個類。當然這段程式碼只是點到即止，諸位可以再做進一步優化。
沒有遵循Sandi Metz的規則
- 每個類程式碼不可以超過100行
- 每個方法程式碼不可以超過5行
- 方法引數不可以超過4個，hash項也包括在內
- 控制器之可以初始化一個物件。而且檢視層只可以使用一個例項變數，並且只可以在這個物件上呼叫方法（@object.collaborator.value這種是不可以的）。
更多關於Sandi Metz的規則請移步至thoughtbot,參閱Sandi Metz’ Rules For Developers這篇博文。