Created
September 27, 2012 09:07
-
-
Save n3dst4/3793015 to your computer and use it in GitHub Desktop.
How to shorten some really wordy code
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
// Original: even without reading the code, it looked wrong. Lots of very | |
// obvious repetition. And switches are so often a code smell. | |
switch (modeName) { | |
case "Manual": | |
alert(self.options.messages.pleaseSet + self.options.messages.Manual); | |
break; | |
case "Automatic": | |
alert(self.options.messages.pleaseSet + self.options.messages.Automatic); | |
break; | |
case "Hybrid": | |
alert(self.options.messages.pleaseSet + self.options.messages.Hybrid); | |
break; | |
} | |
// First pass: get the alert onto the outside of the command, and then just pick | |
// the right message with ternary operators. There's still repetition going on | |
// here though, and I don't like that we've turned "Hybrid" into some sort of | |
// default case here. | |
alert(self.options.messages.pleaseSet + | |
(modeName === "Manual")? self.options.messages.Manual : | |
(modeName === "Automatic")? self.options.messages.Automatic : | |
self.options.messages.Hybrid | |
); | |
// Third time lucky: remember that strings can act as labels! If all you're ever | |
// doing is switching on a string and then finding a member with the same name, | |
// you don't need to write your condition explicitly. Just use [square bracket] | |
// notation instead of .dot notation. | |
alert(self.options.messages.pleaseSet + self.options.messages[modeName]); | |
// This one take an extra line, but makes the construction of the message easier | |
// to read IMO | |
var msg = self.options.messages; | |
alert(msg.pleaseSet + msg[modeName]); | |
// Lastly, maybe you were worried that we've changed the behaviour so that it | |
// will always alert, even if modeName isn't one of the three known values. | |
// Well, I can assure you that in the case where I took this example from, it is | |
// guaranteed to be. Still, this version makes sure there is a message to be | |
// alerted. Not *quite* identical to the original | |
var msg = self.options.messages; | |
if (msg[modeName]) alert(msg.pleaseSet + msg[modeName]); | |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment