Ruby - Extrahiere meine kleineren Methoden aus einer anderen Methode (Umwandlung einer Zeichenkette in zwei Zahlen und ein Symbol)

Question

Ich habe eine Methode (land_connected_rover (Koordinaten)), die einen Parameter (Zeichenfolge, die zwei Zahlen und einen Buchstaben, Leerzeichen getrennt enthält)) end führt Aufforderungen basierend darauf aus, mein Problem ist, dass ich Schwierigkeiten habe, meine Aufgaben von dieser Methode zu extrahieren. Ich habe versucht, private Methoden zu verwenden, aber sobald ich das tue, kann keine meiner injizierten Klassen diese Variablen mehr erreichen. Ich möchte eine elegante, SRP-basierte Lösung haben, nicht die aktuelle unordentliche ... Äh, tut mir leid, dass ich deine Augen verletzt habe!

‚x‘, ‚y‘ und ‚Position‘ Variablennamen für andere Klassen sind wichtig, da sie auf diese Aufforderungen verlassen ...

ich die Schritte extrahieren möchten, die x zuweisen, y und Position variabel Namen auf den rechten Teil der Zeichenfolge.

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 

Ich schätze wirklich die Hilfe! Und tut mir leid, wenn ich alle falschen Fragen gestellt habe, bin ich irgendwie neu ..

Answer 1

Ein Hinweis über opportunity cost. Es ist möglich zu überbewerten. Sich über diese Methode Gedanken zu machen, ist wahrscheinlich nicht die Zeit wert. Es ist eine einfache Methode mit acht Zeilen, die einfach zu verstehen ist und funktioniert. Vermutlich ist es getestet, aber wenn es nicht ist, ist es etwas, wofür Sie Ihre Zeit besser nutzen sollten.

Beispielsweise initialisieren Sie @current_rover zu einer Liste, aber es wird als ein Benutzerobjekt verwendet. Ich vermute, viele Dinge werden auf verwirrende Weise knallen, wenn der Benutzer @current_rover nicht setzt. Das wäre etwas, an dem man Zeit verbringen könnte. Sie könnten Tests haben, die versuchen, ein frisch initialisiertes Objekt zu verwenden und sicherzustellen, dass sie sinnvolle Ausnahmen erzeugen, oder Sie können diese Komplexität umgehen und entscheiden, dass sie mit einem Rover initialisiert werden muss.

Lassen Sie uns das Refactoring als Übung behandeln.

---

Zuerst wollen wir versuchen, all die Dinge zu beschreiben, die land_connected_rover tut, indem jeder Block der Dinge zu kommentieren, zu erklären, was sie tun.

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 

Kommentare wie dieser Punkt an guten reinen Punkten für das Extrahieren von Methoden. Wir brauchen etwas, um Koordinaten zu analysieren. Etwas, um Koordinaten zu setzen. Und etwas, um dem Gitter Koordinaten hinzuzufügen. Wir haben das letzte schon, also extrahieren Sie die anderen beiden.

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 

Jetzt können diese dokumentiert, wiederverwendet und getestet werden. Wenn die Koordinaten nicht analysiert werden, kann Test die Notwendigkeit anzeigen, Fehlerbehandlung hinzuzufügen oder @current_rover wird nicht festgelegt.

Ich verwendete @current_rover, um mit dem Rest des Codes konsistent zu bleiben, der Instanzvariablen für den internen Zugriff anstelle von Zugriffsmethoden verwendet. Es gibt Argumente für beide Wege, wählen Sie einen.

Dann legen Sie sie wieder zusammen.

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

---

Von dort aus können wir ein paar weitere Beobachtungen machen. Warum schreibt der Controller Convenience-Methoden, um Attribute von Rover zu setzen? set_rover_coordinates sollte wahrscheinlich nach Rover umziehen.

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

Wer sollte Koordinaten analysieren? Ich sehe gute Gründe dafür, dass sowohl Rover als auch Controller dies benötigen. Dies deutet darauf hin, dass Sie eine Coordinate-Klasse benötigen.

# 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 

Jetzt beobachten wir eher als eine Roverkoordinaten ein paar Attribute sein, die, sollte es Attribut eine einzige Koordinate, die ein Objekt Koordinaten nimmt.

Jetzt, da wir Koordinatenobjekte haben, kann Controller ein Koordinatenobjekt übergeben werden und sich nicht darum kümmern, seine Eingabe zu normalisieren.

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

Lassen Sie den Anrufer die Normalisierung behandeln.

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

Nun, da Sie Objekte Koordinaten haben, kann der Anrufer nutzen sie, und die meisten der Notwendigkeit für die Konvertierung wird wahrscheinlich verschwinden.

---

Von hier ist meine nächste Sorge die Verdoppelung der Koordinaten zwischen dem Rover und der Oberfläche. Der Rover hat seine eigenen Koordinaten und die Oberfläche scheint die Rover-Koordinaten unabhängig zu verfolgen. Ich nehme an, beide müssen synchron bleiben? Wenn dies der Fall ist, erhöht dies die Komplexität und birgt Risiken. Oder verfolgt die Surface nur, wo der Rover ursprünglich aufsetzte? Es ist etwas zu beachten.

---

Beachten Sie, dass das meiste davon nicht offensichtlich war, bis ich das Refactoring begann. Ich hatte zunächst beim ersten Refactoring von land_connected_rover angehalten, aber dann darüber nachgedacht, kam mit der Coordinate-Klasse, und dann kaskadierte es von dort. Ich denke, das Endergebnis ist viel besser, weit über das, was ich am Anfang erwartet hatte.

Also ... ja, mach dir manchmal Gedanken über die 8-Zeilen-Methode. :)

Answer 2

landete ich mit dieser Lösung am Ende nach oben:

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