clojure-style-guide icon indicating copy to clipboard operation
clojure-style-guide copied to clipboard

Add note about preferring `(if-not (= ...))` over `(if (not= ...))` and `(when-not (= ...))` over `(when (not= ...))`

Open camsaul opened this issue 3 years ago • 8 comments

I was surprised this wasn't already in the style guide. This is a widely-agreed-upon idiom, right? Or am I just imagining things.

camsaul avatar Jul 14 '22 18:07 camsaul

I don't think I've ever heard anyone advocate for this so I wouldn't say it was "widely-agreed" .

Looking at our 140k lines of Clojure, I see just 4 occurrences of (if (not= ..) ..) and 3 of (if-not (= .. ..); I see 39 (when (not= ..) ..) vs 48 (when-not (= ..) ..) -- we have nearly 300 when-not uses altogether.

I think there's an intention preference here around when vs when-not -- when says "I want you to do these things when (this condition) is truthy" and when-not says "I want you to do these things, except when (this condition) is truthy". I think that intention overrides what is in the condition expression itself.

I'm against adding it to the style guide but I'm not exactly in favor of it either.

seancorfield avatar Jul 14 '22 18:07 seancorfield

I agree with @seancorfield here, I wouldn't say I've seen strong consensus either way in the community. I can certainly see an argument for picking one approach in a company's internal style rules, but I'd be more hesitant about adding it to this style guide.

danielcompton avatar Jul 17 '22 21:07 danielcompton

Perhaps the suggestion should be simply to be consistent in the usage of such macros.

bbatsov avatar Aug 03 '22 09:08 bbatsov

FWIW, I don't really agree with the existing if-not and when-not guidelines -- I think they're already over-proscriptive (because of the "intention preference" I mentioned above).

Given the presence already of the guideline to use (not= ..) instead of (not (= ..)) I can see a consistency argument in favor of adding the guidance @camsaul suggests but I don't agree with it in principle, just as I don't agree with the existing guidelines around these macros 😄

In other words, I don't feel strongly one way or the other -- it would just be another part of the style guide that I'd choose to ignore.

seancorfield avatar Aug 03 '22 13:08 seancorfield

Well, those macros are definitely not something particularly important, so no argument from me. The value they add is on the side of less typing, not clearer code. :-)

Might be good to use softer language for less important guidelines (such as https://guide.clojure.style/#when-not) - e.g. "Prefer" instead of "Use" or something along those lines.

bbatsov avatar Aug 03 '22 13:08 bbatsov

On a more meta topic - I think that if-not generally reads poorly and the negative branch should naturally be else branch.

(if-not something
  foo
  bar)

;; same as above, but without the redundant negation
(if something
  bar
  foo)

That has nothing to do with Clojure, though. And I really think that unless would have been a better name than when-not, but that ship has sailed. 😄

bbatsov avatar Aug 03 '22 13:08 bbatsov

I think if-not is more readable when the first branch is much shorter in length than the other branch.

Example:

(if-not something
  foo
  (let [...]
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    bar))

as opposed to

(if something
  (let [...]
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    line-of-code
    bar)
  foo)

the difference is with if-not we get one of the branches out of the way immediately and you can proceed to read the other branch without having to keep the fact that an else branch is pending in mind for ten or twenty lines. It makes for less cognitive load

camsaul avatar Aug 12 '22 02:08 camsaul

Coming back to this nearly a year later: it seems like there's no further input and no clear consensus on the suggested guidelines (although I'm reading "leaning no" overall), so I propose we close this.

seancorfield avatar Jun 10 '23 20:06 seancorfield