這個Ruby類的設計真的很糟糕嗎？

我對OOP相當陌生，我擔心我寫的這門課的設計很差。這似乎違背OOP的幾個原則：這個Ruby類的設計真的很糟糕嗎？

它不包含它自己的數據，而是依賴於對值YAML文件。
它的方法需要但被稱爲在一個特定的順序
它有很多實例變量和方法

它做的工作。它是強大的，但我需要修改源代碼，以在每次添加頁面元素時添加新的getter方法。

它是自動化測試套件中使用的html文檔的模型。我一直在想，有些方法可以放在子類中，但是我擔心我會有太多的類。

您認爲如何？

class BrandFlightsPage < FlightSearchPage 

    attr_reader :route, :date, :itinerary_type, :no_of_pax, 
       :no_results_error_container, :submit_button_element 

    def initialize(browser, page, brand) 
    super(browser, page) 

    #Get reference to config file 
    config_file = File.join(File.dirname(__FILE__), '..', 'config', 'site_config.yml') 

    #Store hash of config values in local variable 
    config = YAML.load_file config_file 

    @brand = brand  #brand is specified by the customer in the features file 

    #Define instance variables from the hash keys 
    config.each do |k,v| 
     instance_variable_set("@#{k}",v) 
    end 

    end 

    def visit 
    @browser.goto(@start_url) 
    end 

    def set_origin(origin) 
    self.text_field(@route[:attribute] => @route[:origin]).set origin 
    end 

    def set_destination(destination) 
    self.text_field(@route[:attribute] => @route[:destination]).set destination 
    end 

    def set_departure_date(outbound) 
    self.text_field(@route[:attribute] => @date[:outgoing_date]).set outbound 
    end 

    def set_journey_type(type) 
    if type == "return" 
     self.radio(@route[:attribute] => @itinerary_type[:single]).set 
    else 
     self.radio(@route[:attribute] => @itinerary_type[:return]).set 
    end 
    end 

    def set_return_date(inbound) 
    self.text_field(@route[:attribute] => @date[:incoming_date]).set inbound 
    end 

    def set_number_of_adults(adults) 
    self.select_list(@route[:attribute] => @no_of_pax[:adults]).select adults 
    end 

    def set_no_of_children(children) 
    self.select_list(@route[:attribute] => @no_of_pax[:children]).select children 
    end 

    def set_no_of_seniors(seniors) 
    self.select_list(@route[:attribute] => @no_of_adults[:seniors]).select seniors 
    end 

    def no_flights_found_message 
    @browser.div(@no_results_error_container[:attribute] => @no_results_error_container[:error_element]).text 
    raise UserErrorNotDisplayed, "Expected user error message not displayed" unless divFlightResultErrTitle.exists? 
    end 

    def submit_search 
    self.link(@submit_button_element[:attribute] => @submit_button_element[:button_element]).click 
    end 
end

來源

2012-08-24 Rogue_Leader

如果這門課被設計成門面，那麼它不是（太）不好的設計。它提供了一種統一的統一方式來執行依賴各種不相關行爲持有者的相關操作。

這似乎是一個糟糕的問題分離，因爲這個類基本上耦合了所有的各種實現細節，這可能會變得有點棘手的維護。

最後，需要以特定順序調用事實方法可能暗示您試圖對狀態機進行建模的事實 - 在這種情況下，它可能應該分解爲幾個類（每個「狀態」）。我不認爲你會達到「太多的方法」或「太多的課程」點，事實是你需要每個班級提供的功能是連貫的和合理的。繪製線的位置取決於您和您的具體實施的域要求。

來源

2012-08-24 10:58:29 Romain

謝謝羅曼。你所說的實際上是我所關心的 - 我正在考慮從BrandFlightsPage進行分類 - 比如BrandFlightsPageAddPorts和BrandFlightsPageAddDates。這將增加健壯性並且允許擴展而不用修改。這將需要修改消耗對象的代碼，但由於這些代碼並沒有分叉或釋放，所以這意味着需要做一些額外的工作。你能理解這個嗎？ –

這可以工作。「BrandFlightsPage」然後將集中所有專用版本之間的通用邏輯。但是，如果可行的話，你應該設法確保呼叫的「順序」約束不能被違反。 – Romain

我認爲這是可行的 - 這是黃瓜自動化測試的支持代碼，所以功能文件和步驟代碼將強制執行順序。謝謝你的幫助。 –

這個Ruby類的設計真的很糟糕嗎？

回答

相關問題