Clojure: idiomatic way of removing duplication in an “if”?

Up vote 3 down vote favorite share g+ share fb share tw.

I'm very new to clojure, and I haven't done a ton of lisp previously. I have a function that contains the following: (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)))))) Obviously, this code is poor because having two almost-identical function calls here is suboptimal, where the only difference is (.indexOf scale degree) vs degree.

What's the Clojure/Lisp Way of removing this code duplication? I feel like it should involve a let, but I'm not positive. Any other general pointers related to this code block are also appreciated.

Edit: I have re-factored the code according to andrew cooke's suggestion, the function now reads: (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)))) ) ) Thanks to everyone who answered so quickly. Clojure refactoring lisp link|improve this question edited 23 hours ago asked yesterdayPaul Sanwald1,325112 91% accept rate.

1 At least for common-lisp (and I assume clojure as well), those last two parentheses are usually placed at the end of the (take...) line; with a good editor that properly indents lisp code, indentation will take the place of what you are doing with those last two parentheses. – claytontstanley 56 mins ago.

Degree) (.indexOf scale degree) degree) (take num_voices (take-nth 2 (cycle (invert scale degree))))) not sure it helps - no general principle, except using let. Also, maybe others wouldn't like the way I shadow values with degree, but here I think it helps show the intent. Edit: compared to other answers, I have pulled out the value.

I prefer this to embedding because I find a long chain of embedded evaluations harder to read. Ymmv. Ps thinking some more this some days later if you are using this style in multiple places (where a parameter can be either a value or a key that pulls data from a preceding value) then I might consider writing a macro to automate that process (ie something that generates a fn with auto-generated let of the form above).

The main problem is deciding how to indicate which paraneters are treated in that way (and also, I would worry about how this could confuse any idea you are using).

Thanks andrew, this seems the easiest to read, for my eye. – Paul Sanwald 23 hours ago.

Degree) (.indexOf scale degree) (invert scale degree)))))))) It would probably be even better if you used a let to capture the result of the if.

Thanks, upvoted, your answer is of course correct but the "let" does make it a bit more readable I think. – Paul Sanwald 23 hours ago @PaulSanwald Yes, that's why my answer says that. – Marcin 22 hours ago.

In Clojure (and most other lisps) if returns a value just like every other expression. For example, (if (even? 3) 1 0) evaluates to 0.

You can use this knowledge to refactor your code by moving the identical portions of the code outside of the if statement, like so: (defn chord scale degree num-voices (take num-voices (take-nth 2 (cycle (invert scale (if (keyword? Degree) (.indexOf scale degree) degree)))))) Also, in Lisp, - isn't special of reserved, so you can and should use it in your variable names. It is better lisp style to use num-voices instead of num_voices or numVoices, as the dashed option is seen as more readable.

Degree) (.indexOf scale degree) degree))))))).

I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.