10
\$\begingroup\$

I'm somewhat new to JavaScript and AngularJS, and read in Doug Crockford's book that hoisted function declarations can lead to issues. If you have any pointers on how to structure my file, those would be much appreciated.

(function() {
    'use strict';

    function PlayerProfileStatsCtrl($q, ColumnService, LookupService, PlayerService, PlayerStatsService) {
       var vm = this;

        var loadStats = function (player) {
            $q.all({
                catchingStats: PlayerStatsService.getCatchingStats(player.playerId),
                fieldingStats: PlayerStatsService.getFieldingStats(player.playerId),
                hittingStats: PlayerStatsService.getHittingStats(player.playerId),
                pitchingStats: PlayerStatsService.getPitchingStats(player.playerId),
                playerStats: LookupService.getByKeyPromise('lkplayerstat'),
                statTypes: LookupService.getListPromise('lkstattype')
            }).then(function (results) {

                // Associate stat result lists with string keyed stat types.
                vm.statResults = {
                    C: results.catchingStats.statList,
                    F: results.fieldingStats.statList,
                    H: results.hittingStats.statList,
                    P: results.pitchingStats.statList
                };

                // Assign the player stat lookup object.
                vm.playerStats = results.playerStats;

                // Assign the stat type lookup list.
                vm.statTypes = results.statTypes; 

                initStatColumnDefs();
            });
        };

        // Call loadStats() after the player promise is resolved.
        vm.player = PlayerService.getPlayer(loadStats);

        var initStatColumnDefs = function () {

            // The column definitions are contained in lists and associated by stat type.
            vm.statColumnDefs = {};

            for (var i = 0; i < vm.statTypes.length; i++) {

                // Get the current stat type key: { 'C', 'F', 'H', 'P' }
               var statTypeKey = vm.statTypes[i].statTypeLk;

               // Get the list of column names from the user.
               var userSelectedStats = ColumnService.getColumns('STATS_' + statTypeKey);

               var getStatColumnDefs = function (userColumns) {
                   var columnDefs = [];

                    // Each stat section will start with these columns.
                    columnDefs.push({ name: 'Season', fullName: 'Season', property: 'statDb.year' });
                    columnDefs.push({ name: 'Team', fullName: 'Team', property: 'team.teamName' });
                    columnDefs.push({ name: 'Level', fullName: 'Level', property: 'level.shortDescription' });

                    // Fielding has an additional column in before custom columns are displayed.
                    if (statTypeKey === 'F') {
                        columnDefs.push({ name: 'Pos', fullName: 'Position', property: 'statDb.positionLk' });
                    }

                    // Construct a custom stat column definition.
                    var getCustomColumnDef = function (userColumn) {
                        var currentStat = vm.playerStats[userColumn];
                        var displayName = currentStat.shortDescription;
                        var onHoverName = currentStat.description;
                        var statProperty = 'statDb.' + currentStat.statProperty;

                        // Format the custom stat according to the player stat lookup table.
                        var getFormattedStat = function (val) {
                            if (val === undefined) {  // Return null if there is no value for that stat.
                                return; 
                            } else {
                                if (!isNaN(val)) {  // Only process numbers.
                                    val = String(val.toFixed(currentStat.precision));

                                    // Remove leading zero.
                                    if (currentStat.leadingZeroFlg && val.startsWith('0')) {
                                        val = val.substr(1, val.length-1);
                                    }

                                     // Add a % sign to the end of the stat.
                                    if (currentStat.percentFlg) {
                                        val += '%';
                                    }
                               }
                               return val;
                           }
                    }
                    return { name: displayName, fullName: onHoverName, property: statProperty, format: getFormattedStat };
                }

                // Add a definition for each of the custom columns.
                for (var i = 0; i < userColumns.length; i++) {
                    columnDefs.push(getCustomColumnDef(userColumns[i]));
                }
                return columnDefs;
            }

            // Store the list of defintions by stat type.
            vm.statColumnDefs[statTypeKey] = getStatColumnDefs(userSelectedStats);

        }
    }
}

 angular
 .module('players')
 .controller('PlayerProfileStatsCtrl', [ '$q', 'ColumnService', 'LookupService', 'PlayerService', 'PlayerStatsService', PlayerProfileStatsCtrl ])
})();
\$\endgroup\$
3
  • 3
    \$\begingroup\$ Note: Crockford says a lot of things. They are not all correct. Function hoisting is probably one of those times. \$\endgroup\$
    – Dan
    Jun 28, 2016 at 15:52
  • \$\begingroup\$ Do you still maintain this code? If so, could you provide context for the PlayerStatsService and LookupService, along with their methods? \$\endgroup\$ Jun 27, 2018 at 22:06
  • \$\begingroup\$ I do not anymore \$\endgroup\$
    – dusk
    Sep 22, 2018 at 15:02

1 Answer 1

6
\$\begingroup\$

Bearing in mind that you claim that you do not to maintain this code anymore1, there are a few suggestions I would have about this code:

  • define functions and accept parameters- that should allow the nested functions to be moved out... You may be able to utilize Partially applied functions.
  • catch rejected promises The code calls $q.all() with 6 promises, yet has no catch() callback! If any one of those promises would fail, should the user be notified?
  • bind function scope instead of assigning vm Utilize Function.prototype.bind() to set the context of this inside callback functions - like the promise callbacks, initStatColumnDefs(), etc. Then there is no need to define the extra variable vm - just use this.
  • Get rid of useless else - There is no need to use the else in the block cited below. If the conditional epxression of the if statement evaluates to true then the return will be reached. Otherwise the rest of the code can be executed normally.

    if (val === undefined) {  // Return null if there is no value for that stat.
         return; 
    } else {
    

1Baseball player statistics controller

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.