2012-04-08 10 views
5

Sono molto nuovo nel clojure e non ho ancora fatto un sacco di lisp in precedenza. Ho una funzione che contiene il seguente:clojure: modo idiomatico di rimuovere la duplicazione in un "se"?

(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)))))) 

Ovviamente, questo codice è scarsa a causa avente due funzione quasi identica chiamate qui è subottimale, dove l'unica differenza è (.indexOf scale degree) vs degree.

Qual è il modo Clojure/Lisp per rimuovere questa duplicazione del codice? Sento che dovrebbe coinvolgere un let, ma non sono positivo. Sono anche apprezzati tutti gli altri suggerimenti generali relativi a questo blocco di codice.

Edit: ho ri-presi il codice secondo il suggerimento di Andrew Cooke, la funzione ora legge:

(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)))) 
     ) 
    ) 

Grazie a tutti coloro che hanno risposto così in fretta.

+2

Almeno per common-lisp (e presumo clojure pure), quelle ultime due parentesi sono di solito poste alla fine della linea (prendere ...); con un buon editor che indentizzi correttamente il codice Lisp, l'indentazione prenderà il posto di quello che stai facendo con quelle ultime due parentesi. –

risposta

6

avrei scritto:

(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))))) 

non sono sicuro che aiuta - nessun principio generale, ad eccezione utilizzando let. inoltre, forse ad altri non piacerebbe il modo in cui i valori dell'ombra sono con degree, ma qui penso che aiuti a mostrare l'intento.

modifica: rispetto ad altre risposte, ho tirato fuori il valore. preferisco l'incorporamento perché trovo più difficile leggere una lunga catena di valutazioni incorporate. YMMV.

ps pensando un po 'di più [questo alcuni giorni dopo] se si utilizza questo stile in più punti (in cui un parametro può essere un valore o una chiave che estrae dati da un valore precedente), quindi potrei prendere in considerazione la scrittura di una macro per automatizzare quel processo (cioè qualcosa che genera un fn con let del modulo sopra generato automaticamente). il problema principale è decidere come indicare quali paranetri sono trattati in quel modo (e inoltre, mi preoccuperei di come questo possa confondere qualsiasi ide che si sta usando).

+0

grazie andrew, questo sembra il più facile da leggere, per il mio occhio. –

6

if restituisce un'espressione, in modo da invertire la struttura della vostra funzione:

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

Probabilmente sarebbe ancora meglio se si è utilizzato un let per catturare il risultato della if.

+0

grazie, upvoted, la tua risposta è corretta, ma il "let" lo rende un po 'più leggibile, penso. –

+0

@PaulSanwald Sì, è per questo che la mia risposta lo dice. – Marcin

4

In Clojure (e la maggior parte delle altre comunicazioni) if restituisce un valore proprio come ogni altra espressione. Ad esempio,

(if (even? 3) 1 0) 

valutazioni a 0.

È possibile utilizzare questa conoscenza per il refactoring il codice spostando le porzioni identiche del codice esterno della dichiarazione if, in questo modo:

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

Inoltre, in Lisp, - non è speciale o riservata, quindi puoi e dovresti usarlo nei nomi delle variabili. È preferibile utilizzare lo stile lisp num-voices anziché num_voices o numVoices, in quanto l'opzione tratteggiata è più leggibile.

0

Non c'è molto che si può fare per semplificare la procedura, forse spostare il if all'interno della chiamata a take num_voices, in questo modo:

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