2012-04-08 2 views
5

Я очень новичок в clojure, и раньше я не делал тонны lisp. У меня есть функция, которая содержит следующее:clojure: идиоматический способ удаления дублирования в «if»?

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

Очевидно, что этот код плох, потому что имеет два почти идентичные функции вызывает здесь неоптимальным, где разница лишь в том (.indexOf scale degree)degree против.

Что такое Clojure/Lisp Способ удаления этого дублирования кода? Я чувствую, что это должно включать в себя пустяк, но я не уверен. Любые другие общие указатели, связанные с этим блоком кода, также оценены.

Edit: Я повторно учитываться код в соответствии с предложением AndreW Cooke, то функция теперь гласит:

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

Спасибо всем, кто ответил так быстро.

+2

По крайней мере, для здравой шепелявости (и я полагаю, Clojure, а также), последние две круглые скобки обычно помещаются в конец строки (взять ...); с хорошим редактором, который правильно отлаживает код lisp, отступ займет место того, что вы делаете с этими двумя последними скобками. –

ответ

6

я бы написать:

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

не уверен, что это не помогает - не общий принцип, за исключением использования let. также, может быть, другим не понравится, как я теневые значения с degree, но здесь я думаю, что это помогает показать намерение.

Редактировать: по сравнению с другими ответами, я вытащил значение. я предпочитаю это встраивать, потому что я нахожу длинную цепочку встроенных оценок труднее читать. YMMV.

ps еще несколько размышлений [это несколько дней спустя], если вы используете этот стиль в нескольких местах (где параметр может быть либо значением, либо ключом, который извлекает данные из предыдущего значения), тогда я мог бы рассмотреть возможность создания макроса для автоматизации этого процесса (т. е. что-то, что генерирует fn с автогенерированным let of the form выше). основная проблема заключается в том, чтобы определить, как указать, какие паранетеры обрабатываются таким образом (а также я буду беспокоиться о том, как это может сбить с толку любой идеал, который вы используете).

+0

спасибо andrew, это кажется самым легким для чтения, для моего глаза. –

6

if возвращает выражение, так инвертировать структуру вашей функции:

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

Это, вероятно, будет даже лучше, если бы вы использовали подведенный, чтобы захватить результат if.

+0

спасибо, ваш ответ, конечно, правильный, но «пусть» делает его более читабельным, я думаю. –

+0

@PaulSanwald Да, вот почему мой ответ говорит об этом. – Marcin

4

В Clojure (и большинстве других lisps) if возвращает значение, как и любое другое выражение. Например,

(if (even? 3) 1 0) 

оценивается в 0.

Вы можете использовать эти знания, чтобы реорганизовать свой код, перемещая одинаковые части кода за пределами if заявления, например, так:

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

Кроме того, в Лиспе, - не является специальным или зарезервирован, поэтому вы можете и должны использовать его в своих именах переменных. Лучше использовать стиль lisp для использования num-voices вместо num_voices или numVoices, так как пунктирная опция рассматривается как более читаемая.

0

Существует не так много, что можно сделать, чтобы упростить процедуру, возможно переместить if внутри вызова take num_voices, как это:

(defn chord ([scale degree num_voices] 
    (take num_voices 
     (take-nth 2 
        (cycle (invert 
          scale 
          (if (keyword? degree) (.indexOf scale degree) degree))))))) 
Смежные вопросы