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

Break complex conditional logic #4

Open
samnang opened this issue May 2, 2012 · 2 comments
Open

Break complex conditional logic #4

samnang opened this issue May 2, 2012 · 2 comments

Comments

@samnang
Copy link
Member

samnang commented May 2, 2012

I see a complex a conditional logic in https://github.com/codereading/rack/blob/rack-1.4/lib/rack/showexceptions.rb#L46

def prefers_plain_text?(env)
  env["HTTP_X_REQUESTED_WITH"] == "XMLHttpRequest" && (!env["HTTP_ACCEPT"] || !env["HTTP_ACCEPT"].include?("text/html"))
end

If I have a long conditional logic like this, I would break each part to make it easy to read:

def prefers_plain_text?(env)
  env["HTTP_X_REQUESTED_WITH"] == "XMLHttpRequest" && 
    !(env["HTTP_ACCEPT"] and env["HTTP_ACCEPT"].include?("text/html"))
end
@jhuckabee
Copy link

@samnang I agree that splitting the line makes it easier to read, however I think the original version of the 2nd conditinal logic reads more cleanly.

I.e.

(!env["HTTP_ACCEPT"] || !env["HTTP_ACCEPT"].include?("text/html"))

vs

!(env["HTTP_ACCEPT"] and env["HTTP_ACCEPT"].include?("text/html"))
  1. Keeping the NOT operator with each condition makes it flow easier when you read it.
  2. I find the use of && and and together inconsistent and distracting. Was that done on purpose?

@samnang
Copy link
Member Author

samnang commented May 2, 2012

Hi @jhuckabee

  1. Keeping the NOT operator with each condition makes it flow easier when you read it.

Sometimes it does that way, but this code it doesn't help because our brain will interpret more ! and ||, that's make it more complex.

  1. I find the use of && and and together inconsistent and distracting. Was that done on purpose?

I don't know most of the time I usually use and and or, but I use && and || whenever I think about operator precedence. So what do you think?

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

2 participants