2012-04-08 1 views
5

Ich bin sehr neu zu Clojure, und ich habe nicht eine Tonne Lispeln zuvor. Ich habe eine Funktion, die Folgendes enthält:clojure: idiomatische Art der Beseitigung von Doppelarbeit in einem "wenn"?

(defn chord 
    ([scale degree num_voices] 
    (if 
     (keyword? degree) 
      (take num_voices (take-nth 2 (cycle (invert scale (.indexOf scale degree))))) 
      (take num_voices (take-nth 2 (cycle (invert scale degree)))))) 

Offensichtlich ist dieser Code ist schlecht, weil mit zwei fast identische Funktion hier nennt, ist suboptimal, wo der einzige Unterschied (.indexOf scale degree) vs degree ist.

Wie ist die Clojure/Lisp-Methode zum Entfernen dieser Code-Duplizierung? Ich habe das Gefühl, es sollte eine Vermietung beinhalten, aber ich bin nicht positiv. Alle anderen allgemeinen Zeiger, die mit diesem Codeblock in Beziehung stehen, werden ebenfalls geschätzt.

Edit: Ich habe den Code nach Andrew Cooke Vorschlag erneut berücksichtigt, liest die Funktion jetzt:

(defn chord 
    ([scale degree num_voices] 
     (let [degree (if (keyword? degree) (.indexOf scale degree) degree)] 
      (take num_voices (take-nth 2 (cycle (invert scale degree)))) 
     ) 
    ) 

Dank an alle, die so schnell beantwortet.

+2

Mindestens für Common-Lisp (und ich nehme an clojure auch), diese letzten zwei Klammern sind normalerweise am Ende der (take ...) Linie platziert; Mit einem guten Editor, der den Lisp-Code korrekt einrückt, wird der Einzug den Platz einnehmen, den Sie mit den letzten beiden Klammern machen. –

Antwort

6

ich würde schreiben:

(defn chord [scale degree num_voices] 
    (let [degree (if (keyword? degree) (.indexOf scale degree) degree)] 
    (take num_voices (take-nth 2 (cycle (invert scale degree))))) 

nicht sicher, ob es hilft - keinen allgemeinen Grundsatz, außer let verwenden. auch, vielleicht würden andere nicht die Art und Weise mag ich Schatten Werte mit degree, aber hier denke ich, es hilft, die Absicht zu zeigen.

edit: im Vergleich zu anderen Antworten habe ich den Wert herausgezogen. Ich bevorzuge das Einbetten, weil ich eine lange Kette von eingebetteten Auswertungen schwerer zu lesen finde. Ymmv.

ps etwas mehr [das einige Tage später] Wenn Sie diesen Stil an mehreren Stellen verwenden (wobei ein Parameter entweder ein Wert oder ein Schlüssel sein kann, der Daten von einem vorhergehenden Wert abruft), könnte ich ein Makro schreiben um diesen Prozess zu automatisieren (dh etwas, das ein Fn mit automatisch generierten Let des obigen Formulars generiert). Das Hauptproblem ist die Entscheidung, wie man angibt, welche Paraneter auf diese Weise behandelt werden (und auch, ich würde mir Sorgen machen, wie dies jede Idee, die du benutzt, verwirren könnte).

+0

danke andrew, das scheint am einfachsten zu lesen, für mein Auge. –

6

if gibt einen Ausdruck, so die Struktur Ihrer Funktion invertieren:

(defn chord 
    ([scale degree num_voices] 
    (take num_voices (take-nth 2 (cycle (invert scale (if (keyword? degree) 
                   (.indexOf scale degree) 
                  (invert scale degree)))))))) 

Es wahrscheinlich sogar besser wäre, wenn Sie eine let verwendet, um das Ergebnis der if zu erfassen.

+0

danke, Upvoted, deine Antwort ist natürlich korrekt, aber das "Let" macht es ein wenig lesbarer, denke ich. –

+0

@PaulSanwald Ja, deshalb sagt meine Antwort das. – Marcin

4

In Clojure (und den meisten anderen Lisps) gibt if einen Wert wie jeden anderen Ausdruck zurück. Zum Beispiel

(if (even? 3) 1 0) 

wertet 0 aus.

Sie dieses Wissen Ihren Code Refactoring durch die identischen Abschnitte des Codes bewegen außerhalb der if Anweisung, wie so verwenden können:

(defn chord [scale degree num-voices] 
    (take num-voices (take-nth 2 
          (cycle (invert scale 
              (if (keyword? degree) 
               (.indexOf scale degree) 
               degree)))))) 

Auch in Lisp, - ist nicht unüblichen oder reserviert, Sie können und sollten es in Ihren Variablennamen verwenden. Es ist besser, den Lisp-Stil num-voices anstelle von num_voices oder numVoices zu verwenden, da die gestrichelte Option besser lesbar ist.

0

Es gibt nicht viel, was getan werden kann, das Verfahren zu vereinfachen, vielleicht die if innerhalb des Anrufs zu take num_voices bewegen, wie folgt aus:

(defn chord ([scale degree num_voices] 
    (take num_voices 
     (take-nth 2 
        (cycle (invert 
          scale 
          (if (keyword? degree) (.indexOf scale degree) degree)))))))