2017-01-24 10 views
2

두 개의 숫자와 문자를 포함하는 문자열, 공백으로 구분 된 매개 변수를 사용하는 메서드 (land_connected_rover (coordinates))가 있습니다.) end는 이것을 기반으로 프롬프트를 실행합니다. 문제는이 메서드에서 할당을 추출하는 데 어려움이 있습니다. 개인 메서드를 시도했지만 일단이 메서드를 사용하면 주입 된 클래스 중 어느 것도이 변수에 더 이상 도달 할 수 없습니다. 나는 우아하고 SRP 기반의 솔루션을 원합니다. 현재 지저분한 것은 아닙니다 ... 예, 눈을 아프게해서 죄송합니다!루비 - 다른 작은 메서드를 추출하는 방법 (문자열을 두 개의 숫자와 기호로 변환)

들이 이러한 프롬프트에 의존으로 'Y'와 '위치'변수 이름은

내가 X를 할당하는 단계를 추출 할 ... 다른 클래스에 중요하다, '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

나쁜 :)하는 로버에게 이러면를 방문하십시오. 아마 당신은 x y와 위치를 설정하는 move_to (x, y)를 만들 수 있습니다. –

답변

3

opportunity cost에 대한 메모. 과도한 인수 요인이 될 수 있습니다. 이 방법에 대한 걱정은 아마도 당신의 시간 가치가 없습니다. 그것은 이해하기 쉽고 작동하는 간단한 문장의 8 줄 방법입니다. 아마도 테스트를 거쳤지 만 그렇지 않은 경우 시간을 보내는 것이 더 좋습니다.

예를 들어 @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 

우리는 몇 가지 더 관찰 할 수 있습니다. 컨트롤러가 Rover의 속성을 설정하는 편리한 방법을 쓰는 이유는 무엇입니까? set_rover_coordinates이 로버로 이동해야합니다.

누가 좌표를 파싱해야합니까? Rover와 Controller 모두에게 필요한 이유를 알 수 있습니다. 이는 Coordinate 클래스가 필요하다는 것을 의미합니다.

# 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 

좌표 개체가 생겼으므로 컨트롤러에 입력 좌표를 전달할 수 있으며 입력 정규화에 대해 걱정할 필요가 없습니다.

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

호출자가 정규화를 처리하도록하십시오.

이제 좌표 개체가 있으므로 호출자가 호출자가 사용할 수 있으며 변환 필요성은 거의 사라집니다.


다음은 로버와 표면 간의 좌표 중복입니다. 누렁이에는 자체 좌표가 있으며 표면은 누렁이의 좌표를 독립적으로 추적합니다. 둘 다 동기화 상태를 유지해야한다고 생각합니까? 그렇다면 복잡성과 위험 요소가 추가됩니다. 아니면 표면이 탐사차가 처음 만진 곳을 추적하고 있을까요? 고려해야 할 사항입니다.


리팩토링을 시작할 때까지는 대부분 명확하지 않았습니다. 처음에 land_connected_rover의 첫 번째 리팩토링에서 멈추었지만 더 이상 그것에 대해 생각하고 Coordinate 클래스를 작성한 다음 거기에서 계단식으로 연결했습니다. 나는 최종 결과가 훨씬 더 좋다고 생각한다. 처음부터 기대했던 것 이상이다.

그래서 ... 가끔은 8 라인 방법에 대해 걱정하십시오. :)

+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 
관련 문제