2017-02-13 4 views
-2

Ich frage mich, was der richtige Weg ist, um diesen Code für die Effizienz zu refaktorieren, neben der doppelten Ausführung.Running-Methode auf zwei Variablen auf einmal

class Hamming 
    def compute (a, b) 

    a.to_a.split("") 
    b.to_a.split("") 

    end 
end 

Gibt es etwas ähnliches auf einmal zwei Variablen wie a, b = 1, 2 zu zuweisen?

+6

Dieser Code macht keinen Sinn. 'Split' arbeitet mit Strings, nicht mit Arrays und es macht nichts mit dem Ergebnis. Wenn du "Effizienz" sagst, fragst du dich, wie wiederholst du dich nicht? – Schwern

+0

Ja. Ich wollte fragen, wie ich es vermeiden kann, mich zu wiederholen. Ich habe nicht einmal daran gedacht zu realisieren, dass du Split auf einem Array nicht ausführen kannst. – Rich

Antwort

1

Da der Code keinen Sinn ergibt, denke ich, was Sie fragen, ist, wie vermeiden Sie es, sich zu wiederholen.

Einfach, schreiben Sie eine andere Methode und rufen Sie das an. Hier ein Beispiel, in dem Sie herausfinden möchten, welche Phrase länger ist, aber Sie möchten viele Leerzeichen ignorieren. So ist foo bar nicht länger als 12345678.

def longer_phrase(phraseA, phraseB) 
    normalizedA = normalize(phraseA) 
    normalizedB = normalize(phraseB) 

    return normalizedA.length > normalizedB.length ? phraseA : phraseB 
end 

def normalize(phrase) 
    normalized = phrase.gsub(/\s+/, ' '); 
    normalized.strip! 

    return normalized 
end 

puts longer_phrase("foo   bar ", "12345678") 

Bevor Sie mit der Arbeit beginnen, müssen Sie alle Daten normalisieren. Dies vermeidet es, sich selbst zu wiederholen. Es macht Ihren Code einfacher zu verstehen, da wir wissen, was der Sinn all dieser Arbeit ist, die Zeichenfolge zu normalisieren. Und es gibt Ihnen eine Normalisierungsfunktion, die Sie woanders verwenden können, um Ihre Daten auf die gleiche Weise zu normalisieren.

2

Zunächst ist Ihr Code ungültig. #to_a gibt ein Array zurück; #split ist nicht für Arrays definiert.

Zweitens, wenn Ihr Code war gültig (sagen wir, a.to_s.split(""); b.to_s.split(""), wäre es nicht tatsächlich viel tun, weil Ihr Code würde den Wert der zuletzt ausgeführten Anweisung nur zurückgeben (b.to_s.split("")). Beide #to_s und #split sind nicht -destructive, was bedeutet, dass sie nicht a oder b ändern - der einzige Effekt, den Sie von dieser Funktion erhalten ist, was es gibt, und Sie nicht zurück das Ergebnis a.to_s.split("") in irgendeiner Weise: es ist vergessen

Wenn Sie gemeint. etwas wie dieses:

class Hamming 
    def compute(a, b) 
    [ 
     a.to_s.split(""), 
     b.to_s.split("") 
    ] 
    end 
end 

das ist ziemlich lesbar. wenn Sie komplexere Operation hatte jedoch nicht nur .to_s.split(""), wäre es besser, es in seine eigene Funktion zu isolieren:

class Hamming 
    def compute(a, b) 
    [ 
     list_chars(a), 
     list_chars(b) 
    ] 
    end 
    private def list_chars(str) 
    str.to_s.split("") 
    end 
end 

Sie vereinfachen könnte es sogar map mehr verwenden, aber es ist wirklich wird nur dann erforderlich, wenn Sie mehrere haben Elemente, wie der Zwei-Elemente-Fall ist perfekt lesbar, wie es ist. Doch hier geht:

class Hamming 
    def compute(a, b) 
    [a, b].map { |x| list_chars(x) } 
    end 
    private def list_chars(str) 
    str.to_s.split("") 
    end 
end 

Auch Sie könnten die Methode #each_char, so dass Sie einen Iterator sehen möchten, die besser lesbar ist, und oft die richtige Wahl, als .split("").

EDIT: Nachdem Sie ein wenig darüber nachgedacht haben, scheint es, als ob Sie eine Methode starten, um eine Hamming-Distanz zwischen zwei Saiten zu bewerten; und dass Sie nicht beabsichtigen, diese Funktion zu haben, geben Sie einfach das Zeichen der zwei Zeichenfolgen zurück. In diesem Fall würde ich dies nur schreiben:

def compute(a, b) 
    a_chars = a.to_s.each_char 
    b_chars = b.to_s.each_char 
    # ... 
end 

oder dies möglicherweise, wenn Sie unbedingt Zeichen haben sich brauchen, und kein Iterator:

def compute(a, b) 
    a_chars = a.to_s.each_char.to_a 
    b_chars = b.to_s.each_char.to_a 
    # ... 
end 

Die Lösung, die ich Ihnen glauben suchen für würde so aussehen:

def compute(a, b) 
    a_chars, b_chars = *[a, b].map { |x| x.to_s.each_char.to_a } 
    # ... 
end 

aber ich würde das weniger lesbar als die nicht-DRY eine betrachten; wenn Sie wirklich trocknen Sie es wollen nach oben, extrahieren Sie die listification in seine eigene Funktion, wie oben beschrieben, und tun nur

a_chars = list_chars(a) 
b_chars = list_chars(b) 

die eigentlich das Beste aus beiden Welten, auch wenn es ein bisschen zuviel des Guten in dieser ist Fall: Es ist DRY-ly wartbar und selbst-dokumentarisch lesbar, für ein bisschen Kompromiss in Ausführlichkeit.

Verwandte Themen