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

clean code for switch condition #5596

Closed
geraldsamosir opened this issue Apr 12, 2024 · 2 comments
Closed

clean code for switch condition #5596

geraldsamosir opened this issue Apr 12, 2024 · 2 comments
Labels

Comments

@geraldsamosir
Copy link

geraldsamosir commented Apr 12, 2024

i want to ask about this code

that write with logic

  switch (lc) {
    case 'referer':
    case 'referrer':
      return this.headers.referrer
        || this.headers.referer;
    default:
      return this.headers[lc];
  }
};

is that more readable and cleaner when we write that with

  switch (lc) {
    case 'referer':
      return  this.headers.referer;
    case 'referrer':
      return this.headers.referrer;
    default:
      return this.headers[lc];
  }
};

i just giving my opinion as express js users

@wesleytodd
Copy link
Member

Thanks for your opinion. This code has been there for a while and I don't know if folks shared style opinions back when it was written. But we do not accept style only PRs, so likely there is not much we are going to change for this.

@LinusU
Copy link
Member

LinusU commented Apr 12, 2024

Just giving some more context here: the proposed change would no longer do the same thing. The point of the code is to treat both the referrer and referer header as the same. In your updated version only the one asked for would be given, making the entire switch case functionally equivalent to just return this.headers[lc].

Here is some background on why referrer is misspelled in the header name: https://en.wikipedia.org/wiki/HTTP_referer#Etymology

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

No branches or pull requests

3 participants