Baseball player statistics controller

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

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

StackExchange Code Review Q#133295, answer score: 6