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

Refactor parseArgs function in dialog module #5436

Merged
merged 3 commits into from
May 8, 2016

Conversation

stevekinney
Copy link
Contributor

I did a little bit of refactoring on the parseArgs helper function in the dialog module.

  • When checking the last argument to see if it's a function, we access the array twice. I cached the argument into the variable in order to prevent that additional array access.
  • I added destructuring for the argument shifting.
  • In the conditional check to see if the first argument was a BrowserWindow, we were checking to see if window was null and then immediately checking it again in the ternary. We were also flipping the entire conditional after the fact. My thinking is that we could short-circuit using && and save the extra conditional check and then just check the constructor if it wasn't null. (I wonder if one call to Object.getPrototypeOf(window) would be a better approach?)

I wrote up some tests to verify that this all worked as I was going along, but parseArgs is a private helper and I didn't want to expose it as a public method, but I'm happy to figure out a way to include them if that's helpful.

@zcbenz
Copy link
Member

zcbenz commented May 8, 2016

Looks perfect to me ❤️ .

@zcbenz zcbenz merged commit 1dcbd35 into electron:master May 8, 2016
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

Successfully merging this pull request may close these issues.

None yet

2 participants