这个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类的设计真的很糟糕吗？

回答

相关问题