Single-branch if shouldn't be replaced by when. #2

Open
technomancy opened this Issue Mar 4, 2012 · 7 comments

Comments

Projects
None yet
6 participants
@technomancy
Contributor

technomancy commented Mar 4, 2012

The readme says that if it finds the code (if (some test) (some action) nil) it will suggest replacing it with while. I think it means when, but this isn't right; single-branched if forms are totally acceptable if the focus is on the return value. Since when has an implicit do form, it is a way of signaling to readers that side-effects are involved, while if forms are all about what value is returned. Instead consider adding a rule for replacing (if cond (do some action)) with when.

@AlexBaranosky

This comment has been minimized.

Show comment
Hide comment
@AlexBaranosky

AlexBaranosky Mar 4, 2012

Maybe what we really need is an easy way to configure which substitutions different users prefer?

Maybe what we really need is an easy way to configure which substitutions different users prefer?

@cemerick

This comment has been minimized.

Show comment
Hide comment
@cemerick

cemerick Sep 28, 2012

Contributor

The battle continues. ;-) +1 for @AlexBaranosky's suggestion.

Contributor

cemerick commented Sep 28, 2012

The battle continues. ;-) +1 for @AlexBaranosky's suggestion.

@jonase

This comment has been minimized.

Show comment
Hide comment
@jonase

jonase Sep 28, 2012

Owner

Is this rule causing problems? Are you getting pull requests from users who have run kibit on your project and you don't agree with the changes (I've seen this happen)? If that's the case I'd rather just remove the rule for now until I find time to revisit this project.

Owner

jonase commented Sep 28, 2012

Is this rule causing problems? Are you getting pull requests from users who have run kibit on your project and you don't agree with the changes (I've seen this happen)? If that's the case I'd rather just remove the rule for now until I find time to revisit this project.

@cemerick

This comment has been minimized.

Show comment
Hide comment
@cemerick

cemerick Sep 28, 2012

Contributor

My apologies; I saw this issue on my way to creating #52, so perhaps the +1 wasn't especially valuable.

Contributor

cemerick commented Sep 28, 2012

My apologies; I saw this issue on my way to creating #52, so perhaps the +1 wasn't especially valuable.

@technomancy

This comment has been minimized.

Show comment
Hide comment
@technomancy

technomancy Nov 20, 2012

Contributor

@jonase yeah, I would like to use Kibit on Leiningen, but the amount of noise generated from this bug makes it annoying to try it out.

Contributor

technomancy commented Nov 20, 2012

@jonase yeah, I would like to use Kibit on Leiningen, but the amount of noise generated from this bug makes it annoying to try it out.

@Raynes

This comment has been minimized.

Show comment
Hide comment
@Raynes

Raynes Nov 26, 2012

Contributor

I strongly disagree that this is a 'bug' of any sort. I personally prefer when for single branch ifs and would be disappointed if this got removed entirely.

Contributor

Raynes commented Nov 26, 2012

I strongly disagree that this is a 'bug' of any sort. I personally prefer when for single branch ifs and would be disappointed if this got removed entirely.

@danielcompton

This comment has been minimized.

Show comment
Hide comment
@danielcompton

danielcompton Nov 10, 2014

Collaborator

I'm inclined to agree with @technomancy on this. I propose removing single-branch if -> when rules, but making them available through add-on custom rules #66. Thoughts?

Collaborator

danielcompton commented Nov 10, 2014

I'm inclined to agree with @technomancy on this. I propose removing single-branch if -> when rules, but making them available through add-on custom rules #66. Thoughts?

@ikitommi ikitommi referenced this issue in metosin/reitit Jun 6, 2018

Merged

0.1.2 #96

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment