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

DRY entry point #370

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 28 additions & 35 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
var os = require('os');
var utils = require('./lib/utils');

// All notifiers
var NotifySend = require('./notifiers/notifysend');
var NotificationCenter = require('./notifiers/notificationcenter');
Expand All @@ -10,40 +7,36 @@ var WindowsBalloon = require('./notifiers/balloon');

var options = { withFallback: true };

var osType = utils.isWSL() ? 'WSL' : os.type();

switch (osType) {
case 'Linux':
module.exports = new NotifySend(options);
module.exports.Notification = NotifySend;
break;
case 'Darwin':
module.exports = new NotificationCenter(options);
module.exports.Notification = NotificationCenter;
break;
case 'Windows_NT':
if (utils.isLessThanWin8()) {
module.exports = new WindowsBalloon(options);
module.exports.Notification = WindowsBalloon;
} else {
module.exports = new WindowsToaster(options);
module.exports.Notification = WindowsToaster;
}
break;
case 'WSL':
module.exports = new WindowsToaster(options);
module.exports.Notification = WindowsToaster;
break;
default:
if (os.type().match(/BSD$/)) {
module.exports = new NotifySend(options);
module.exports.Notification = NotifySend;
} else {
module.exports = new Growl(options);
module.exports.Notification = Growl;
}
function getOsType () {
var utils = require('./lib/utils');
var osType = require('os').type();

if (osType.match(/BSD$/)) {
osType === 'BSD';
} else if (osType === 'Windows_NT' && utils.isLessThanWin8()) {
osType === 'Windows_7_and_Below';
} else if (utils.isWSL()) {
osType = 'WSL';
}

return osType.toLowerCase();
Copy link
Author

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.

}

var osMap = {
bsd: NotifySend,
darwin: NotificationCenter,
linux: NotifySend,
windows_7_and_below: WindowsBalloon,
windows_nt: WindowsToaster,
wsl: WindowsToaster
};
Copy link
Author

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.


var osNotifier = osMap[getOsType()] || Growl;
Copy link
Author

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;
Copy link
Author

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


// Expose notifiers to give full control.
module.exports.NotifySend = NotifySend;
module.exports.NotificationCenter = NotificationCenter;
Expand Down