Skip to content

Commit

Permalink
Refactor cond->> into an easier to read if
Browse files Browse the repository at this point in the history
  • Loading branch information
@RickMoynihan
RickMoynihan committed May 4, 2018
1 parent bee245b commit a0da97d
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/lein_tools_deps/plugin.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
absolute file, using the base to to form the absolute file if needed."
[base-path path]
(let [file (io/file path)]
(cond->> file (not (.isAbsolute file)) (io/file base-path))))
(if (not (.isAbsolute file))

This comment has been minimized.

Sign in to view

Copy link
@aisamu

aisamu May 7, 2018

Hi!
Thanks for this library!
If readability is the concern, is there a reason you didn't use if-not?
(Or even switching the branches, which would make the not unnecessary.)
I assume you have broader concerns that I can't pinpoint with my limited experience!

This comment has been minimized.

Sign in to view

Copy link
@RickMoynihan

RickMoynihan May 7, 2018

Author Owner

Hi,

There are no broader concerns, other than that there's nothing fancy in this code to warrant using a cond->>, which should really be used for cases where there are more than two branches and you want to emphasise the commonality of the last argument across branches. Its use here actually momentarily confused me, and at first I thought the code was doing something more involved than it really is. Hence I thought it clearer to use a simpler form, that wouldn't make me stop and think.

I did consider swapping the branches, to drop the negation but tend to prefer the primary case to be on the truthy branch. I didn't spend more than a few seconds looking at the code, so yes if-not would be preferable still; but there's nothing at all wrong with (if (not ,,,) ,,, ,,,).
(io/file base-path file)
file)))

(defn absolute-path
"Takes an absolute base path and a potentially relative file and returns an
Expand Down

0 comments on commit a0da97d

Please sign in to comment.