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

Remove ternary within if else to see logic error better #63

Open
osv opened this issue Nov 7, 2019 · 2 comments
Open

Remove ternary within if else to see logic error better #63

osv opened this issue Nov 7, 2019 · 2 comments

Comments

@osv
Copy link

osv commented Nov 7, 2019

Common guys, don't use ternary operator, replace it within if else to see logic errors more faster, next error is too childish -_-

Example:
https://github.com/chromium/permission.site/blob/master/index.js#L108

    "camera": function() {
      navigator.mediaDevices ?
        navigator.mediaDevices.getUserMedia(
          {video: true}).then(
            displayOutcome("camera", "success"),
            displayOutcome("camera", "error")
        ) :
        navigator.getUserMedia(
          {video: true},
          displayOutcome("camera", "success"),
          displayOutcome("camera", "error")
        );
    },

Uncaught TypeError: navigator.getUserMedia is not a function

And same in lot of other places

xonx4l added a commit to xonx4l/permission.site that referenced this issue Aug 23, 2023
An solution for issue -:chromium#63
@xonx4l
Copy link

xonx4l commented Aug 23, 2023

so I replaced with if else . If its okay then I can replace others too waiting for your feedback .

@lgarron
Copy link
Collaborator

lgarron commented Aug 23, 2023

Common guys, don't use ternary operator, replace it within if else to see logic errors more faster, next error is too childish -_-

Although I would suggest finding a way to word your suggestions more constructively with the future and don't think ternary is an issue here, I agree that the braces of the if statement are a bit less likely to lead to issues if someone is working on the code in the future.

However, we can make the code ever easier to maintain by making it more DRY. Since the function calls are the same, the most idiomatic choice could be to use nullish coalescing:

(navigator.mediaDevices ?? navigator).getUserMedia(
  {video: true}).then(
    displayOutcome("camera", "success"),
    displayOutcome("camera", "error")
)

However, since we don't transpile the code we should stick to older syntax for maximum compatibility with older browser:

(navigator.mediaDevices || navigator).getUserMedia(
  {video: true}).then(
    displayOutcome("camera", "success"),
    displayOutcome("camera", "error")
)

At that point, I would recommend reusing the calculation as well:

var getUserMediaThis = navigator.mediaDevices || navigator; // Place below the existing `navigator.getUserMedia` fallback assignment.
var getUserMedia = getUserMediaThis.getUserMedia.bind(getUserMediaThis);

// …

getUserMedia(
  {video: true}).then(
    displayOutcome("camera", "success"),
    displayOutcome("camera", "error")
)

I'll leave the choice to @engedy.

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

3 participants