Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
camsaul opened this issue Jul 14, 2022 · 8 comments

Comments

@camsaul
Copy link

camsaul commented Jul 14, 2022

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 camsaul changed the title Prefer (if-not (= ...)) over (if (not= ...)) and (when-not (= ...)) over (when (not= ...)) Add note about preferring (if-not (= ...)) over (if (not= ...)) and (when-not (= ...)) over (when (not= ...)) Jul 14, 2022
@seancorfield
Copy link
Collaborator

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.

@danielcompton
Copy link
Collaborator

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.

@bbatsov
Copy link
Owner

bbatsov commented Aug 3, 2022

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

@seancorfield
Copy link
Collaborator

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.

@bbatsov
Copy link
Owner

bbatsov commented Aug 3, 2022

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
Copy link
Owner

bbatsov commented Aug 3, 2022

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. 😄

@camsaul
Copy link
Author

camsaul commented Aug 12, 2022

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

@seancorfield
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants