-
Notifications
You must be signed in to change notification settings - Fork 322
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
DRY entry point #370
base: master
Are you sure you want to change the base?
DRY entry point #370
Conversation
There was a simple repeating pattern that could be made more DRY with a map.
osType = 'WSL'; | ||
} | ||
|
||
return osType.toLowerCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Store all of the logic for determining the OS in one function. Keeps all the related code together. Also easier for unit testing if you ever decide to do that. May want to move this to the utils file.
windows_7_and_below: WindowsBalloon, | ||
windows_nt: WindowsToaster, | ||
wsl: WindowsToaster | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert the giant Switch block over to a simple object that maps the the only two unique pieces of information, the OS and the notifier to use.
wsl: WindowsToaster | ||
}; | ||
|
||
var osNotifier = osMap[getOsType()] || Growl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get the correct OS, use that to get the correct notifier, or default to Growl.
|
||
// Set the notifier specific to the current OS | ||
module.exports = new osNotifier(options); | ||
module.exports.Notification = osNotifier; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer repeated 7 times
There was a simple repeating pattern that could be made more DRY with a map.