2017-01-24 47 views
2

我有一個方法(land_connected_rover(coordinates)),它需要一個參數(包含兩個數字和一個字母的字符串,空格分隔)end在此基礎上執行提示,我的問題是我努力從這個方法中提取我的任務,我嘗試了私有方法,但是一旦我這樣做了,我的注入類就不會再有這些變量了。我想擁有一個優雅的,基於SRP的解決方案,而不是當前混亂的解決方案......後悔,傷害你的眼睛!Ruby - 從另一種方法中提取我的小方法(將字符串轉換爲兩個數字和一個符號)

「X」,「Y」和「位置」變量名是其他類重要的,因爲它們依賴於這些提示...

我想提取被分配X的步驟,y和位置變量名稱到字符串的右側部分。

class Controller 

    attr_reader :current_rover, :current_surface 

    def initialize 
    @current_surface = nil 
    @current_rover ||= [] 
    end 

    def connect_to_surface(destination) 
    @current_surface = destination 
    end 

    def connect_to_rover(rover) 
    @current_rover = rover 
    end 

    def land_connected_rover(coordinates) 
    coordinates = coordinates.delete(' ') 
    x = coordinates[0].to_i 
    y = coordinates[1].to_i 
    position = coordinates[2].to_sym 
    self.current_rover.x_coordinates = x 
    self.current_rover.y_coordinates = y 
    self.current_rover.position = position 
    add_to_grid(x,y) 
    end 

    def navigate(command) 
    self.current_rover.read_input(command) 
    end 

    private 

    def add_to_grid(x,y) 
    @current_surface.record_on_map(x,y) 
    end 

    def turn_on_rover 
    @current_rover.online = true 
    end 
end 

我真的很感謝幫助!對不起,如果我問了所有錯誤的問題,我有點新...

+0

着陸流動站這個心不是那麼糟糕:)。可能你可以做一個move_to(x,y),它設置x y和位置 –

回答

3

關於opportunity cost的說明。有可能超過因素。擔心這種方法可能不值得你花時間。這是簡單陳述的八行方法,很容易理解並且可行。據推測,它已經過測試,但如果不是,那就更好地花時間。

例如,您將@current_rover初始化爲一個列表,但它用作用戶對象。如果用戶沒有設置@current_rover,我懷疑很多事情會以混亂的方式發生。這將花費時間。您可以嘗試使用新初始化對象的測試,並確保它們產生有意義的異常,或者您可以側重所有複雜性並決定必須使用流動站進行初始化。

讓我們將重構視爲一個練習。


首先,讓我們試着通過評論的事情每個塊來解釋他們所做的事情來描述所有land_connected_rover做的事情。

def land_connected_rover(coordinates) 
    # Parse coordinates. 
    coordinates = coordinates.delete(' ') 
    x = coordinates[0].to_i 
    y = coordinates[1].to_i 
    position = coordinates[2].to_sym 

    # Set rover coordinates. 
    self.current_rover.x_coordinates = x 
    self.current_rover.y_coordinates = y 
    self.current_rover.position = position 

    # Add something to the grid for some reason. 
    add_to_grid(x,y) 
    end 

像這樣的評論在提取方法的好處很多。我們需要一些東西來解析座標。設置座標的東西。還有一些東西可以將座標添加到網格中。我們已經有了最後一個,所以提取另外兩個。

def parse_coordinates(input) 
     input = input.delete(' ') 

     return { 
      x: coordinates[0].to_i, 
      y: coordinates[1].to_i, 
      position: coordinates[2].to_sym 
     }; 
    end 

    def set_rover_coordinates(coordinates) 
     @current_rover.x_coordinates = coordinates[:x] 
     @current_rover.y_coordinates = coordinates[:y] 
     @current_rover.position = coordinates[:position] 
    end 

現在可以對這些文件進行記錄,重用和單元測試。當座標不解析時,測試可能會顯示需要添加錯誤處理,或者未設置@current_rover

我使用@current_rover以保持與使用實例變量進行內部訪問而非訪問者的其他代碼保持一致。無論哪種方式都有爭議,選一個。

然後把它們放回到一起。

def land_connected_rover(input) 
    set_rover_coordinates(parse_coordinates(input)) 
    add_to_grid(x,y) 
    end 

從那裏,我們可以做一些更多的觀察。爲什麼Controller編寫便捷的方法來設置Rover的屬性? set_rover_coordinates應該轉移到羅孚。

def land_connected_rover(input) 
    @current_rover.set_coordinates(parse_coordinates(input)) 
    add_to_grid(x,y) 
    end 

誰應該解析座標?我可以看到羅孚和控制器都需要這個的充分理由。這表明你需要一個座標類。

# In Controller 
    def land_connected_rover(input) 
    @current_rover.set_coordinates(Coordinate.from_a(input)) 
    add_to_grid(x,y) 
    end 

    # In Rover 
    def set_coordinates(coordinates) 
    @x_coordinates = coordinates.x 
    @y_coordinates = coordinates.y 
    @position = coordinates.position 
    end 

現在我們觀察到,而不是流動站的座標是一堆的屬性,它應該有一個單一的座標屬性,需要一個協調的對象。

# In Rover 
    attr :coordinates 

    # In Controller 
    def land_connected_rover(input) 
    @current_rover.coordinates(Coordinate.from_a(input)) 
    add_to_grid(x,y) 
    end 

既然我們有座標對象,Controller可以傳遞一個座標對象,而不用擔心標準化它的輸入。

def land_connected_rover(coordinates) 
    @current_rover.coordinates(coordinates) 
    add_to_grid(x,y) 
    end 

讓調用者處理歸一化。

controller.land_connected_rover(Coordinate.from_a([10,20,30])) 

既然您有座標對象,調用者可以使用它們,並且大多數轉換需求可能會消失。


從這裏我的下一個問題是漫遊車和地面之間的座標重複。羅孚擁有自己的座標,而Surface似乎獨立跟蹤羅孚的座標。我假設兩者都必須保持同步?如果是這樣,那會增加複雜性並且會增加錯誤。或者,Surface可能只追蹤羅孚最初接觸的地方?這是需要考慮的事情。


請注意,直到我開始重構時,大部分情況並不明顯。我最初在land_connected_rover的第一次重構時停下了腳步,但後來想到了一些,提出了Coordinate類,然後它從那裏級聯。我認爲最終的結果要好得多,遠遠超出我一開始預期的結果。

所以......是的,有時候會擔心八行方法。 :)

+1

非常感謝你花費在回答上的幫助和時間,非常感謝你。我結束了更短的一些,但我的面試成功了,再次感謝! :) –

0

我結束了這個解決方案在最後:

class Controller 

    attr_reader :current_rover, :current_surface 

    def initialize 
    @current_surface ||= nil 
    @current_rover ||= nil 
    end 

    def connect_to_surface(destination) 
    @current_surface = destination 
    end 

    def connect_to_rover(rover) 
    @current_rover = rover 
    end 

    def is_mission_ready? 
    @current_surface != nil && @current_rover != nil 
    end 

    def land_connected_rover(coordinates) 
    raise 'Mission is not ready to launch yet!' if !is_mission_ready? 
    x, y, position = parse_coordinates(coordinates) 
    current_rover.x_coordinates = x 
    current_rover.y_coordinates = y 
    current_rover.position = position 
    add_to_grid 
    end 

    def navigate(command) 
    raise 'Navigation is not ready to start yet!' if !is_mission_ready? 
    current_rover.read_input(command) 
    end 

    private 

    def parse_coordinates(coordinates) 
    fields = coordinates.split(" ") 
    [fields[0].to_i, fields[1].to_i, fields[2].to_sym] 
    end 

    def add_to_grid 
    x = current_rover.x_coordinates 
    y = current_rover.y_coordinates 
    position = current_rover.position 
    @current_surface.record_on_map(x,y) 
    puts "Rover landed, currently facing: #{current_rover.position}, x-coordinates: #{x}, y-coordinates: #{y}" 
    @current_surface.grid 
    end 
end 
相關問題