如何處理Rails/Ruby中瑣碎的「重複代碼」異味

因此，我們都努力減少重複（DRY）和其他異味，並儘可能保持我們的代碼儘可能地乾淨整潔。對於Ruby代碼，有很多工具可以檢測氣味，例如相當不錯的Caliber服務。如何處理Rails/Ruby中瑣碎的「重複代碼」異味

但是，似乎我對代碼重複的定義不同於工具。我認爲這可能與Ruby的做事方式有關，你幾乎從不直接訪問變量，而是通過方法調用。從Rails控制器考慮這個片斷：

def update_site_settings 
    SiteSettings.site_name = params[:site_name] 
    SiteSettings.site_theme = params[:site_theme] 
    expire_fragment('layout_header') 
    flash[:notice] = t(:Site_settings_updated) 
    redirect_to :controller => 'application', :action => 'edit_site_settings' 
end

這被標記，因爲兩次調用「PARAMS」的方法與代碼重複警告。所以我的問題是，將params分配給本地變量真的是一種改進嗎？我認爲這樣寫的方式是最明確和簡潔的方式，而事實上，在Ruby中，params是一種方法而不是變量，這很簡單「做生意的成本」。

我看到這是錯誤的方式嗎？

編輯：在這種情況下，一種更漂亮的方式可能是做一個SiteSettings.update_attributes(params)樣式更新。在另一個片段考慮，如果你願意，同樣的問題：

def update 
    @mailing_list = MailingList.find(params[:id]) 

    if @mailing_list.update_attributes(params[:mailing_list]) 
    flash[:notice] = t:Mailing_list_updated 
    redirect_to(mailing_lists_path) 
    ...

來源

2010-02-20 Jakob Borg

謝謝你們。我將以「使用氣味報告作爲指導，並且不會爲每一個細節出汗」作爲一般答案。 – 2010-02-20 13:07:35

我想你也可以聲明Feature Envy代碼異味的一個小實例，雖然它確實有些微不足道。

或許可以引入郵件列表上一個類的方法，所以在控制器方法變得像

def update 
    if @mailing_list = MailingList.update_attributes_by_id(params) 
    ... 

class MailingList 
    def self.update_attributes_by_id(params) 
    id = params.delete(:id) 
    find(id).update_attributes(params) 
    ...

（未測試，所以小心輕放）

我會在現實生活中的煩惱呢？可能不會，因爲一件事情是兩部分的查找/更新是非常常見的，以至於人們立即理解它 - 即使你有一個完美表達的名字，像上面這樣編碼的人也不得不停下來思考一下。

這些分析器（我在自己的代碼上運行Kevin Rutherford的reek）很好，但他們不瞭解上下文，所以他們很少提供完美的信息。它們對於識別可能從注意受益的領域非常有用，但會有很多誤報，並且他們也會錯過任何東西，所以需要在意識中使用它們。

來源

2010-02-20 12:12:32

有一點要記住的DRY和代碼味道的概念，他們是準則。他們在那裏幫助您思考如何在更多的樹木意義上組織和簡化您的代碼。雖然總是牢記這些概念是很好的，但挑剔的代碼重複性往往會導致代碼中出現不必要的複雜性或模糊不清。你的代碼的可理解性也應該很重要，就像你說的那樣，有時代碼最清晰簡潔的地方與最後一次重複的代碼被刪除的代碼是不一樣的。

來源

2010-02-20 12:06:08

如何處理Rails/Ruby中瑣碎的「重複代碼」異味

回答

相關問題