CodeSOD: Switch Kicker

As covered previously, game code generally doesn't represent an interesting WTF. But bad e-commerce code certainly does. So today, Rhys sends us some JavaScript from a web-based fantasy-football (for non-USA values of football) site. Given that it handles microtransactions and in-game items passed between players, it's definitely more on the e-commerce side of things than anything else.

And much like that previous article, this one does involve a very large switch, but before we get to the switch, we have to get to the state validator:

 else if(propertyName == "initUser" || propertyName == "updateUser" || propertyName == "submitLogin" || propertyName == "initRequestPasswordForm" || propertyName == "submitPasswordRequest" || propertyName == "resetPassowrd" || propertyName == "submitGameLogin" || propertyName == "deleteAlertReturn" || propertyName == "submitRegistration" || propertyName == "submitGameRegistration" || propertyName == "acceptGiftReturn" || propertyName == "sellGiftReturn" || propertyName == "buildTicker" || propertyName == "submitAward" || propertyName == "submitGameAward" || propertyName == "buyAsset" || propertyName == "sellAsset" || propertyName == "giftAsset" || propertyName == "loadArcadeProgress" || propertyName == "getGameCompositeAward" || propertyName == "visitFriend" || propertyName == "createGuestUser" || propertyName == "isOnBoard" || propertyName == "processCompletedCompetition" || propertyName == "handleNewPasswordSent" || propertyName == "resetPassword" || propertyName == "submitFriendMail" || propertyName == "savePitchPosition" || propertyName == "cardGiftConfirmed-noOverlay" || propertyName == "startCompetitionAfterCRNUpload" || propertyName == "handleReferralThanks" || propertyName == "loadPriamStore" || propertyName == "buyPriamItem") { handlePostRender(propertyName, data); }

Now, they're validating inputs, which is good, but the pile of magic strings begs for being turned into constants (and thus helping avoid input errors in the first place), or some sort of array check or something to make this more readable.

But inside the handlePostRender we process these inputs, which also highlights how not necessary this if statement even is:

 case "openStore": if(tutorial.hasStarted) if(tutorial.current == 5) { ajaxEngine.googleTrackEvent("event", {category:"pre-registration", action:"enter-store", label:"set-up-process"}); tutorial.next(); // hide close button $(".close-button").css("display", "none"); // append tutorial to overlay $('#tutorial').clone().appendTo('#Award-shopfront-overlay'); $("#Award-shopfront-overlay").find("#tutorial-6").show(); } setupStoreButtons(); break; case "loadFriends": populateFriendList(assetsObject); break; case "loadFriendsAfterAdd": populateFriendList(assetsObject, true); break; case "addFriend": populateFriendList(assetsObject); break; case "joinInvitation": handleJoinInvitation(assetsObject); break; case "loadStore": populateStore(assetsObject); break; case "initUser": setupAwardDetails(assetsObject); break; case "updateUser": updateAwardDetails(assetsObject); break; case "loadGiftChoose-appendOverlay-opaqueBack": populateGiftFriendList(assetsObject); break; case "loadCardGiftChoose-appendOverlay-opaqueBack": populateCardGiftFriendList(assetsObject); break; case "customiseAward": populateAward(assetsObject); break; case "customiseGameAward-noOverlay": populateGameAward(assetsObject); break; case "giftConfirmed-noOverlay": showGiftSent(assetsObject); break; case "cardGiftConfirmed-noOverlay": showCardGiftSent(assetsObject); break; case "processMessage": displayMail(assetsObject,-1); break; case "login": initLogin(); break; case "submitLogin": //loaderfeedback.hide(); handleSubmitLogin(assetsObject); break; case "submitPasswordRequest": handleSubmitPasswordRequest(assetsObject); break; case "initRequestPasswordForm": initRequestPasswordForm(assetsObject); break; case "resetPassword": account.resetPassword(); break; case "accountDeleted": account.accountDeleted(); break; case "handleNewPasswordSent": handleNewPasswordSent(assetsObject); break; case "submitGameLogin": handleGameLogin(assetsObject); break; case "submitLogout-noOverlay": handleLogout(); break; case "submitLogout-noReload": // do nought, this is handled earlier // in AjaxEngine::makeRequest break; case "deleteAlertReturn": handleDeleteAlertReturn(assetsObject); break; case "register": initRegister(); break; case "submitRegistration": handleSubmitRegister(assetsObject); break; case "submitGameRegistration": handleGameRegister(assetsObject); break; case "acceptGiftReturn": handleAcceptGiftReturn(assetsObject); break; case "sellGiftReturn": handleSellGiftReturn(); break; case "pollReturn": handlePollReturn(); break case "predictionReturn": handlePredictionReturn(); break case "buildTicker": handleTickerTape(assetsObject); break; case "processNews": handleNewsItems(assetsObject); break; case "submitAward": handleAwardSubmitted(assetsObject); break; case "submitGameAward": handleGameAwardSubmitted(assetsObject); break; case "buildPlayerDetails": populatePlayerDetails(assetsObject); break; case "buildPlayerBio": populatePlayerBio(assetsObject); break; case "competitionReturn-noOverlay": handleCompetitionPromoReturn(assetsObject); break case "processCompletedCompetition": handleCompetitionFormReturn() //handleProcessCompletedCompetition(); break; case "processCompetitionForm": handleCompetitionFormReturn(assetsObject); break; case "submitCompetitionCollectedData": handleCompetitionFormReturn(assetsObject); break; case "sellAsset": handleStoreSell(); // When selling item, make sure the turf world // is re-built (and inited, particularly) setupAwardDetails(assetsObject.turf); break; case "buyAsset": handleStoreBuy(); break; case "giftAsset": handleStoreGive(); break; case "handleUnwear": handleUnwear(); break; case "handleWear": handleWear(); break; case "quizReturn-noOverlay": handleQuizPromoReturn(assetsObject); break; case "processQuizForm": handleQuizFormReturn(assetsObject); break; case "printPlayers-noOverlay": handlePrintingPlayerCards(assetsObject); break; case "handlePrintAllTrophiesAndMedals-noOverlay": handlePrintAllTrophiesAndMedals(assetsObject); break; case "handlePrintAllTrophies-noOverlay": handlePrintAllTrophies(assetsObject); break; case "handlePrintAllMedals-noOverlay": handlePrintAllMedals(assetsObject); break; case "printInvite-noOverlay": handlePrintInvite(assetsObject); break; case "loadArcadeProgress": handleArcadeLoaded(assetsObject); break; case "getGameCompositeAward": handleGameCompositeAward(assetsObject); break; case "visitFriend": handleVisitFriend(assetsObject); break; case "createGuestUser": handleCreateGuestUser(assetsObject); break; case "isOnBoard": handleOnBoarding(assetsObject); break; case "submitFriendMail": handleFriendRequestAccepted(assetsObject); break; case "savePitchPosition" : startApp(assetsObject); break; case "handleTrophyOverlay" : handleTrophyOverlay(); break; case "loadMedal" : handleMedalsOverlay(); break; case "setupSaveProfile": account.setupSaveProfile(); break; case "processFinalisedQuiz": processFinalisedQuiz(); break; case "startCompetitionAfterCRNUpload" : processCompetitionCRNThanks(); break; case "handleReferralThanks" : handleSubmitRegisterReferral(assetsObject); break; case "loadPriamStore" : populatePriamStore(assetsObject); break; case "buyPriamItem" : handleBoughtPriamItem(assetsObject); break;

As you might gather here, this repeats all the same validation, but also dispatches to actual actions. Rhys suggests a solution which would likely be close to my own: an object acting as a lookup table with callback functions as the values, e.g.:

let action = { //... "loadPriamStore": () => populatePriamStore(assetsObject), "buyPriamItem": () => handleBoughtPriamItem(assetsObject)}if (propertyName in action) action[propertyName]();

The original code was probably written in an older dialect of JavaScript, but the basic principle is viable, with perhaps some minor modifications to properly enclose the data required by the callbacks, or maybe ways to break the actions into categories and have different lookup tables for different categories of action, so that nothing gets too gigantic. One can nitpick the solutions, but the one that doesn't involve a nearly 300 line switch is probably the right one.

Rhys adds: "I'm able to share this because on the live site, none of the code is minified or obfuscated."

[Advertisement] ProGet's got you covered with security and access controls on your NuGet feeds. Learn more.