如何用Rspec重構Rails模型中的複雜方法？

我有以下複雜的方法。我試圖找到並實施可能的改進。現在我把最後一條語句移到Access類。如何用Rspec重構Rails模型中的複雜方法？

def add_access(access) 
    if access.instance_of?(Access) 
    up = UserAccess.find(:first, :conditions => ['user_id = ? AND access_id = ?', self.id, access.id]) 
    if !up && company 
     users = company.users.map{|u| u.id unless u.blank?}.compact 
     num_p = UserAccess.count(:conditions => ['user_id IN (?) AND access_id = ?', users, access.id]) 
     if num_p < access.limit 
     UserAccess.create(:user => self, :access => access) 
     else 
     return "You have exceeded the maximum number of alotted permissions" 
     end 
    end 
    end 
end

我想在重構之前添加規格。我添加了第一個。應該怎樣看起來像別人？

describe "#add_permission" do 
    before do 
     @permission = create(:permission) 
     @user = create(:user) 
    end 

    it "allow create UserPermission" do 
     expect { 
     @user.add_permission(@permission) 
     }.to change { 
     UserPermission.count 
     }.by(1) 
    end 
    end

來源

2012-10-30 tomekfranek

這種方法很複雜，因爲您的模型關係很複雜。這些模型是什麼以及它們爲什麼/如何相互作用？ – pje

這裏是我會怎麼做。

使訪問更像初始斷言，並提出錯誤，如果發生。

建立一個新的方法來檢查現有的用戶訪問 - 這看起來是可重用的，並且更具可讀性。

然後，公司限制更像是對我的驗證，將其作爲自定義驗證移動到UserAccess類。

class User 

    has_many :accesses, :class_name=>'UserAccess' 

    def add_access(access) 
    raise "Can only add a Access: #{access.inspect}" unless access.instance_of?(Access) 

    if has_access?(access) 
     logger.debug("User #{self.inspect} already has the access #{access}") 
     return false 
    end 

    accesses.create(:access => access) 
    end 

    def has_access?(access) 
    accesses.find(:first, :conditions => {:access_id=> access.id}) 
    end 

end 

class UserAccess 

    validate :below_company_limit 

    def below_company_limit 
    return true unless company 
    company_user_ids = company.users.map{|u| u.id unless u.blank?}.compact 
    access_count = UserAccess.count(:conditions => ['user_id IN (?) AND access_id = ?', company_user_ids, access.id]) 
    access_count < access.limit 
    end 

end

來源

2012-10-30 20:00:58

我假設有一個關聯UserAccess，或者你可以添加一個。 –

我還建議將'num_p'重命名爲'number_of_permissions'和'users'以'company_users_ids'。當然，確保你正在測試你的代碼，它使得像這樣的重構成爲令人難以置信的快樂體驗！ – wpp

好點 - 我留下了這一點，所以代碼來自哪裏更清楚，但你是對的。編輯以反映您的更改。 –

您對此課程有單元和/或集成測試嗎？我會在重構之前先寫一些。

假設你有測試，第一個目標可能是縮短這個方法的長度。

這裏有一些改進，使：

移動UserAccess.find調用UserAccess模型，使之命名範圍。
同樣，也移動count方法。

每次更改後重新測試並保持提取狀態，直到乾淨。每個人對清潔都有不同的看法，但是當你看到它時你就會知道。

來源

2012-10-30 19:48:12

+1鼓勵測試！測試，然後重構，然後測試。沖洗並重復。 – thisfeller

其他想法，不相關的移動代碼，但仍然清潔：

users = company.users.map{|u| u.id unless u.blank?}.compact 
num_p = UserAccess.count(:conditions => ['user_id IN (?) AND access_id = ?', users, access.id])

能成爲：

num_p = UserAccess.where(user_id: company.users, access_id: access.id).count

來源

2012-10-30 19:51:34

如何用Rspec重構Rails模型中的複雜方法？

回答

相關問題