From 5c33e89581317da07944ebedef15d08fec958a69 Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Thu, 12 Oct 2017 21:24:06 +0200 Subject: [PATCH 1/7] scores: Make ranking independent of score's .stage property, as it is not present in the scores.json version of it. --- src/js/services/ng-scores.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/js/services/ng-scores.js b/src/js/services/ng-scores.js index fad72e58..bc07647c 100644 --- a/src/js/services/ng-scores.js +++ b/src/js/services/ng-scores.js @@ -618,13 +618,15 @@ define('services/ng-scores',[ }); } - // Convert number of stages to take to a number (i.e. Infinity when - // e.g. `true` is passed) + // Ensure the number of rounds to use for each stage is a + // number, and is not higher than the configured number of + // rounds per stage. // And create empty lists for each stage var board = {}; Object.keys(stageFilter).forEach(function (stage) { var s = stageFilter[stage]; - stageFilter[stage] = typeof s === "number" && s || s && Infinity || 0; + var maxRounds = typeof s === "number" && s || s && Infinity || 0; + stageFilter[stage] = Math.min(maxRounds, $stages.get(stage).rounds); board[stage] = []; }); @@ -662,7 +664,7 @@ define('services/ng-scores',[ } } if (!bteam) { - var maxRounds = Math.min(s.stage.rounds, stageFilter[s.stageId]); + var maxRounds = stageFilter[s.stageId]; var initialScores = new Array(maxRounds); var initialEntries = new Array(maxRounds); for (i = 0; i < maxRounds; i++) { From b47464b69c58b528e947b55380da05cdadbf5a5c Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Mon, 16 Oct 2017 15:49:13 +0200 Subject: [PATCH 2/7] scores: Make ranking independent of score's .team property, as it is not present in the scores.json version of it. --- src/js/services/ng-scores.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/js/services/ng-scores.js b/src/js/services/ng-scores.js index bc07647c..a5400585 100644 --- a/src/js/services/ng-scores.js +++ b/src/js/services/ng-scores.js @@ -658,7 +658,7 @@ define('services/ng-scores',[ var i; var bstage = board[s.stageId]; for (i = 0; i < bstage.length; i++) { - if (bstage[i].team.number === s.team.number) { + if (bstage[i].teamNumber === s.teamNumber) { bteam = bstage[i]; break; } @@ -672,7 +672,7 @@ define('services/ng-scores',[ initialEntries[i] = null; } bteam = { - team: s.team, + teamNumber: s.teamNumber, scores: initialScores, rank: null, highest: null, @@ -737,7 +737,7 @@ define('services/ng-scores',[ // extra criterion. // Note: team number's might be strings, so don't assume numeric // compare is possible. - result = (teamEntry1.team.number > teamEntry2.team.number) ? 1 : -1; + result = (teamEntry1.teamNumber > teamEntry2.teamNumber) ? 1 : -1; } return result; } @@ -777,6 +777,18 @@ define('services/ng-scores',[ }); } + // Convert team number into team object + for (var stageId in board) { + if (!board.hasOwnProperty(stageId)) { + continue; + } + var stage = board[stageId]; + stage.forEach(function (teamEntry) { + teamEntry.team = $teams.get(teamEntry.teamNumber); + delete teamEntry.teamNumber; + }); + } + return board; }; From eb35a98fddc033280e762a27ea7f20b99535fe93 Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Mon, 16 Oct 2017 15:57:54 +0200 Subject: [PATCH 3/7] scores: Move board init out of stageFilter preparation. --- src/js/services/ng-scores.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/js/services/ng-scores.js b/src/js/services/ng-scores.js index a5400585..c0cc34cc 100644 --- a/src/js/services/ng-scores.js +++ b/src/js/services/ng-scores.js @@ -621,13 +621,10 @@ define('services/ng-scores',[ // Ensure the number of rounds to use for each stage is a // number, and is not higher than the configured number of // rounds per stage. - // And create empty lists for each stage - var board = {}; Object.keys(stageFilter).forEach(function (stage) { var s = stageFilter[stage]; var maxRounds = typeof s === "number" && s || s && Infinity || 0; stageFilter[stage] = Math.min(maxRounds, $stages.get(stage).rounds); - board[stage] = []; }); // Create filtered scores (both user-supplied filter and errors). @@ -650,6 +647,12 @@ define('services/ng-scores',[ return true; }); + // Create empty lists for each requested stage + var board = {}; + Object.keys(stageFilter).forEach(function (stage) { + board[stage] = []; + }); + // Convert all valid scores to a per-stage array of objects // per team (containing team and entries per round) filteredScores.forEach(function (s) { From c5ce064f06db1d39e35db44ec6e5f60e00639151 Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Wed, 25 Oct 2017 21:46:50 +0200 Subject: [PATCH 4/7] ng-scores: Refactor calculateScoreboard and isValidScore out into common code (no functional changes). --- karma.conf.js | 1 + spec/common/rankingSpec.js | 155 +++++++++++++++++++++++ spec/services/ng-scoresSpec.js | 24 ---- src/js/common/ranking.js | 221 +++++++++++++++++++++++++++++++++ src/js/services/ng-scores.js | 171 +------------------------ 5 files changed, 381 insertions(+), 191 deletions(-) create mode 100644 spec/common/rankingSpec.js create mode 100644 src/js/common/ranking.js diff --git a/karma.conf.js b/karma.conf.js index c17c80ac..d03ced6a 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -21,6 +21,7 @@ module.exports = function(config) { 'src/components/idbwrapper/idbstore.js', 'spec/helpers/*.js', 'src/js/factories/*.js', + 'src/js/common/*.js', 'src/js/directives/*.js', 'src/js/services/*.js', 'src/js/views/*.js', diff --git a/spec/common/rankingSpec.js b/spec/common/rankingSpec.js new file mode 100644 index 00000000..b6343862 --- /dev/null +++ b/spec/common/rankingSpec.js @@ -0,0 +1,155 @@ +describe('ranking', function () { + var ranking; + beforeEach(function () { + ranking = factory('common/ranking'); + }); + + describe('isValidScore', function () { + it('should accept valid scores', function () { + expect(ranking.isValidScore(0)).toBe(true); + expect(ranking.isValidScore(-1)).toBe(true); + expect(ranking.isValidScore(1000)).toBe(true); + expect(ranking.isValidScore("dnc")).toBe(true); // Did Not Compete + expect(ranking.isValidScore("dsq")).toBe(true); // DiSQualified + }); + + it('should reject invalid scores', function () { + expect(ranking.isValidScore(undefined)).toBe(false); + expect(ranking.isValidScore(null)).toBe(false); + expect(ranking.isValidScore(NaN)).toBe(false); + expect(ranking.isValidScore(Infinity)).toBe(false); + expect(ranking.isValidScore(-Infinity)).toBe(false); + expect(ranking.isValidScore("dnq")).toBe(false); + expect(ranking.isValidScore("foo")).toBe(false); + expect(ranking.isValidScore(true)).toBe(false); + expect(ranking.isValidScore(false)).toBe(false); + expect(ranking.isValidScore({})).toBe(false); + expect(ranking.isValidScore([])).toBe(false); + }); + }); + + describe('calculateScoreboard', function () { + it('should output used stages', function () { + var board = ranking.calculateScoreboard([], { test: 1 }); + expect(Object.keys(board)).toEqual(['test']); + }); + + it('should fill in all rounds for a team', function () { + // If a team has played at all (i.e., they have a score for that stage) + // then all other rounds for that team need to have an entry (which can + // be null). + var board = ranking.calculateScoreboard([ + { teamNumber: 41, stageId: 'test', round: 2, score: 10, published: true } + ], { test: 3 }); + expect(board['test'][0].scores).toEqual([null, 10, null]); + }); + + it('should rank number > dnc > dsq > null', function () { + var board = ranking.calculateScoreboard([ + { teamNumber: 41, stageId: 'test', round: 1, score: 'dsq', published: true }, + { teamNumber: 42, stageId: 'test', round: 1, score: 'dnc', published: true }, + { teamNumber: 43, stageId: 'test', round: 1, score: -1, published: true }, + { teamNumber: 44, stageId: 'test', round: 1, score: 1, published: true }, + ], { test: 3 }); + var result = board['test'].map(function (entry) { + return { + rank: entry.rank, + teamNumber: entry.teamNumber, + highest: entry.highest + }; + }); + expect(result).toEqual([ + { rank: 1, teamNumber: 44, highest: 1 }, + { rank: 2, teamNumber: 43, highest: -1 }, + { rank: 3, teamNumber: 42, highest: 'dnc' }, + { rank: 4, teamNumber: 41, highest: 'dsq' }, + ]); + + }); + + it("should assign equal rank to equal scores", function () { + var board = ranking.calculateScoreboard([ + { teamNumber: 41, stageId: 'test', round: 1, score: 10, published: true }, + { teamNumber: 41, stageId: 'test', round: 2, score: 20, published: true }, + { teamNumber: 41, stageId: 'test', round: 3, score: 30, published: true }, + { teamNumber: 42, stageId: 'test', round: 1, score: 30, published: true }, + { teamNumber: 42, stageId: 'test', round: 2, score: 10, published: true }, + { teamNumber: 42, stageId: 'test', round: 3, score: 20, published: true }, + { teamNumber: 43, stageId: 'test', round: 1, score: 30, published: true }, + { teamNumber: 43, stageId: 'test', round: 2, score: 0, published: true }, + { teamNumber: 43, stageId: 'test', round: 3, score: 20, published: true }, + ], { test: 3 }); + var result = board['test'].map(function (entry) { + return { + rank: entry.rank, + teamNumber: entry.teamNumber, + highest: entry.highest + }; + }); + // Note: for equal ranks, teams are sorted according + // to (ascending) team id + expect(result).toEqual([ + { rank: 1, teamNumber: 41, highest: 30 }, + { rank: 1, teamNumber: 42, highest: 30 }, + { rank: 2, teamNumber: 43, highest: 30 }, + ]); + }); + + it("should allow filtering rounds", function () { + var board = ranking.calculateScoreboard([ + { teamNumber: 41, stageId: 'test', round: 1, score: 10, published: true }, + { teamNumber: 41, stageId: 'test', round: 2, score: 20, published: true }, + { teamNumber: 41, stageId: 'test', round: 3, score: 30, published: true }, + { teamNumber: 42, stageId: 'test', round: 1, score: 30, published: true }, + { teamNumber: 42, stageId: 'test', round: 2, score: 10, published: true }, + { teamNumber: 42, stageId: 'test', round: 3, score: 20, published: true }, + { teamNumber: 43, stageId: 'test', round: 1, score: 30, published: true }, + { teamNumber: 43, stageId: 'test', round: 2, score: 0, published: true }, + { teamNumber: 43, stageId: 'test', round: 3, score: 20, published: true }, + ], { test: 2 }); + var result = board['test'].map(function (entry) { + return { + rank: entry.rank, + teamNumber: entry.teamNumber, + scores: entry.scores + }; + }); + // Note: for equal ranks, teams are sorted according + // to (ascending) team id + expect(result).toEqual([ + { rank: 1, teamNumber: 42, scores: [30, 10] }, + { rank: 2, teamNumber: 43, scores: [30, 0] }, + { rank: 3, teamNumber: 41, scores: [10, 20] }, + ]); + }); + + it("should exclude scores for unknown rounds / stages", function () { + var board = ranking.calculateScoreboard([ + { teamNumber: 41, stageId: 'foo', round: 1, score: 0, published: true }, + { teamNumber: 41, stageId: 'test', round: 0, score: 0, published: true }, + { teamNumber: 41, stageId: 'test', round: 4, score: 0, published: true }, + ], { test: 3 }); + expect(board['test'].length).toEqual(1); + }); + + it("should ignore invalid scores", function () { + var board = ranking.calculateScoreboard([ + { teamNumber: 41, stageId: 'test', round: 1, score: "foo", published: true }, + { teamNumber: 41, stageId: 'test', round: 2, score: NaN, published: true }, + { teamNumber: 41, stageId: 'test', round: 3, score: Infinity, published: true }, + { teamNumber: 42, stageId: 'test', round: 1, score: {}, published: true }, + { teamNumber: 42, stageId: 'test', round: 2, score: true, published: true }, + ], { test: 3 }); + expect(board['test'].length).toEqual(0); + }); + + it("should include/overwrite duplicate score", function () { + var board = ranking.calculateScoreboard([ + { teamNumber: 41, stageId: 'test', round: 1, score: 10, published: true }, + { teamNumber: 41, stageId: 'test', round: 1, score: 20, published: true }, + ], { test: 3 }); + // Last score will overwrite any previous entry + expect(board['test'][0].highest).toEqual(20); + }); + }); +}); diff --git a/spec/services/ng-scoresSpec.js b/spec/services/ng-scoresSpec.js index 40667328..ddb72b16 100644 --- a/spec/services/ng-scoresSpec.js +++ b/spec/services/ng-scoresSpec.js @@ -235,30 +235,6 @@ describe('ng-scores',function() { }); }); - describe('isValidScore', function () { - it('should accept valid scores', function () { - expect($scores.isValidScore(0)).toBe(true); - expect($scores.isValidScore(-1)).toBe(true); - expect($scores.isValidScore(1000)).toBe(true); - expect($scores.isValidScore("dnc")).toBe(true); // Did Not Compete - expect($scores.isValidScore("dsq")).toBe(true); // DiSQualified - }); - - it('should reject invalid scores', function () { - expect($scores.isValidScore(undefined)).toBe(false); - expect($scores.isValidScore(null)).toBe(false); - expect($scores.isValidScore(NaN)).toBe(false); - expect($scores.isValidScore(Infinity)).toBe(false); - expect($scores.isValidScore(-Infinity)).toBe(false); - expect($scores.isValidScore("dnq")).toBe(false); - expect($scores.isValidScore("foo")).toBe(false); - expect($scores.isValidScore(true)).toBe(false); - expect($scores.isValidScore(false)).toBe(false); - expect($scores.isValidScore({})).toBe(false); - expect($scores.isValidScore([])).toBe(false); - }); - }); - describe('scoreboard', function() { var board; beforeEach(function() { diff --git a/src/js/common/ranking.js b/src/js/common/ranking.js new file mode 100644 index 00000000..e4ee86da --- /dev/null +++ b/src/js/common/ranking.js @@ -0,0 +1,221 @@ +/** + * Ranking calculation used on server and client. + */ + +// UMD boilerplate (modified from https://github.com/umdjs/umd/blob/master/templates/commonjsAdapter.js) +if (typeof exports === 'object' && typeof exports.nodeName !== 'string' && typeof define !== 'function') { + var define = function (name, factory) { + factory(require, exports, module); + }; +} +define("common/ranking", function (require, exports, module) { + + /** + * Determine whether given score is valid, i.e. a number, or "dnc" (Did Not Compete), + * or "dsq" (DiSQualified). + * Note: `null` and `undefined` are invalid: remove the score to denote this instead. + * + * @param score {any} Score value to test + * @return true when score is valid + */ + function isValidScore(score) { + return typeof score === "number" && score > -Infinity && score < Infinity || + score === "dnc" || score === "dsq"; + } + + /** + * Compute rankings based on given scores and a stageFilter. + * Note that only published scores will be included in the resulting + * output. + * + * Scores is an array of score objects, where each score object should + * have the following properties: + * @typedef {Object} Score + * @property teamNumber {number} + * @property teamNumber {number} + * @property stageId {string} + * @property round {number} + * @property score {number | string} + * @property published {boolean} + * + * StageFilter is an object of stageId => numberOfRounds: + * @type {Object} StageFilter + * + * The result is an object with the rankings per stageId, + * where each ranking is an array of rank objects. + * A rank object has the following properties: + * @typedef {Object} Rank + * @property rank {number} + * @property teamNumber {number} + * @property scores {Array.} + * @property sortedScores {Array.} + * @property entries {Array.} + * @property highestScore {number} + * + * @param scores {Array.} Scores + * @param stageFilter {StageFilter} Hash of stageId => numberOfRounds + * @return {Array.} Ranking based on filtered scores + */ + function calculateScoreboard(scores, stageFilter) { + if (typeof stageFilter !== "object") { + throw new TypeError("stageFilter expected"); + } + // Create filtered scores (both user-supplied filter and errors). + var filteredScores = scores.filter(function (s) { + // Only include published scores + if (!s.published) { + return false; + } + + // Ignore completely invalid scores + if (!isValidScore(s.score)) { + return false; + } + + // Ignore score if filtered + if (!stageFilter[s.stageId] || s.round > stageFilter[s.stageId]) { + return false; + } + + return true; + }); + + // Create empty lists for each requested stage + var board = {}; + Object.keys(stageFilter).forEach(function (stage) { + board[stage] = []; + }); + + // Convert all valid scores to a per-stage array of objects + // per team (containing team and entries per round) + filteredScores.forEach(function (s) { + // Find existing entry for this team, or create one + var bteam; + var i; + var bstage = board[s.stageId]; + for (i = 0; i < bstage.length; i++) { + if (bstage[i].teamNumber === s.teamNumber) { + bteam = bstage[i]; + break; + } + } + if (!bteam) { + var maxRounds = stageFilter[s.stageId]; + var initialScores = new Array(maxRounds); + var initialEntries = new Array(maxRounds); + for (i = 0; i < maxRounds; i++) { + initialScores[i] = null; + initialEntries[i] = null; + } + bteam = { + teamNumber: s.teamNumber, + scores: initialScores, + rank: null, + highest: null, + entries: initialEntries, + }; + bstage.push(bteam); + } + bteam.scores[s.round - 1] = s.score; + bteam.entries[s.round - 1] = s; + }); + + // Compares two scores. + // Returns 0 if scores are equal, 1 if score2 is larger than score1, + // -1 otherwise. + // Note: this ordering causes Array.sort() to sort from highest to lowest score. + function scoreCompare(score1, score2) { + if (score1 === score2) { + return 0; + } + var comp = false; + if (score1 === null || score2 === null) { + comp = (score1 === null); + } else if (score1 === "dsq" || score2 === "dsq") { + comp = (score1 === "dsq"); + } else if (score1 === "dnc" || score2 === "dnc") { + comp = (score1 === "dnc"); + } else if (typeof score1 === "number" && typeof score2 === "number") { + comp = score1 < score2; + } else { + throw new TypeError("cannot compare scores '" + score1 + "' and '" + score2 + '"'); + } + return comp ? 1 : -1; + } + + // Compares two scores-arrays. + // Returns 0 if arrays are equal, 1 is scores2 is larger than scores1, + // -1 otherwise. + // Note: this ordering causes Array.sort() to sort from highest to lowest score. + function scoresCompare(scores1, scores2) { + var result = 0; + var i; + if (scores1.length !== scores2.length) { + throw new RangeError("cannot compare score arrays with different number of rounds"); + } + for (i = 0; i < scores1.length; i++) { + result = scoreCompare(scores1[i], scores2[i]); + if (result !== 0) + break; + } + return result; + } + + // Compare two 'team entries' (members of a scoreboard stage). + // 1 is teamEntry2 has higher scores than teamEntry1, or -if scores are + // equal- teamEntry1 has a higher team number. Returns -1 otherwise. + // Note: this ordering causes Array.sort() to sort from highest to lowest score, + // or in ascending team-id order. + function entryCompare(teamEntry1, teamEntry2) { + var result = scoresCompare(teamEntry1.sortedScores, teamEntry2.sortedScores); + if (result === 0) { + // Equal scores, ensure stable sort by introducing + // extra criterion. + // Note: team number's might be strings, so don't assume numeric + // compare is possible. + result = (teamEntry1.teamNumber > teamEntry2.teamNumber) ? 1 : -1; + } + return result; + } + + function createSortedScores(teamEntry) { + teamEntry.sortedScores = teamEntry.scores.slice(0); // create a copy + teamEntry.sortedScores.sort(scoreCompare); + teamEntry.highest = teamEntry.sortedScores[0]; + } + + function calculateRank(state, teamEntry) { + if (state.lastScores === null || scoresCompare(state.lastScores, teamEntry.sortedScores) !== 0) { + state.rank++; + } + state.lastScores = teamEntry.sortedScores; + teamEntry.rank = state.rank; + return state; + } + + // Sort by scores and compute rankings + for (var stageId in board) { + if (!board.hasOwnProperty(stageId)) { + continue; + } + var stage = board[stageId]; + + // Create sorted scores and compute highest score per team + stage.forEach(createSortedScores); + + // Sort teams based on sorted scores + stage.sort(entryCompare); + + // Compute ranking, assigning equal rank to equal scores + stage.reduce(calculateRank, { + rank: 0, + lastScores: null + }); + } + + return board; + } + + exports.isValidScore = isValidScore; + exports.calculateScoreboard = calculateScoreboard; +}); diff --git a/src/js/services/ng-scores.js b/src/js/services/ng-scores.js index c0cc34cc..d0c37e6c 100644 --- a/src/js/services/ng-scores.js +++ b/src/js/services/ng-scores.js @@ -5,11 +5,12 @@ define('services/ng-scores',[ 'services/ng-services', 'services/log', + 'common/ranking', 'services/ng-fs', 'services/ng-stages', 'factories/poller', 'services/ng-teams', -],function(module,log) { +],function(module, log, ranking) { "use strict"; // Current file version for scores. @@ -235,19 +236,6 @@ define('services/ng-scores',[ this._update(); }; - /** - * Determine whether given score is valid, i.e. a number, or "dnc" (Did Not Compete), - * or "dsq" (DiSQualified). - * Note: `null` and `undefined` are invalid: remove the score to denote this instead. - * - * @param score {any} Score value to test - * @return true when score is valid - */ - Scores.prototype.isValidScore = function (score) { - return typeof score === "number" && score > -Infinity && score < Infinity || - score === "dnc" || score === "dsq"; - }; - /** * Convert 'dirty' score value to correct type used during score * computations etc. @@ -549,7 +537,7 @@ define('services/ng-scores',[ // mean that one could 'reset' a team's score for that round. // If a team did not play in a round, there will simply be no // entry in scores. - if (!self.isValidScore(s.score)) { + if (!ranking.isValidScore(s.score)) { s.error = new InvalidScoreError(s.score); return; } @@ -627,158 +615,7 @@ define('services/ng-scores',[ stageFilter[stage] = Math.min(maxRounds, $stages.get(stage).rounds); }); - // Create filtered scores (both user-supplied filter and errors). - var filteredScores = this.scores.filter(function (s) { - // Only include published scores - if (!s.published) { - return false; - } - - // Ignore completely invalid scores - if (!self.isValidScore(s.score)) { - return false; - } - - // Ignore score if filtered - if (!stageFilter[s.stageId] || s.round > stageFilter[s.stageId]) { - return false; - } - - return true; - }); - - // Create empty lists for each requested stage - var board = {}; - Object.keys(stageFilter).forEach(function (stage) { - board[stage] = []; - }); - - // Convert all valid scores to a per-stage array of objects - // per team (containing team and entries per round) - filteredScores.forEach(function (s) { - // Find existing entry for this team, or create one - var bteam; - var i; - var bstage = board[s.stageId]; - for (i = 0; i < bstage.length; i++) { - if (bstage[i].teamNumber === s.teamNumber) { - bteam = bstage[i]; - break; - } - } - if (!bteam) { - var maxRounds = stageFilter[s.stageId]; - var initialScores = new Array(maxRounds); - var initialEntries = new Array(maxRounds); - for (i = 0; i < maxRounds; i++) { - initialScores[i] = null; - initialEntries[i] = null; - } - bteam = { - teamNumber: s.teamNumber, - scores: initialScores, - rank: null, - highest: null, - entries: initialEntries, - }; - bstage.push(bteam); - } - bteam.scores[s.round - 1] = s.score; - bteam.entries[s.round - 1] = s; - }); - - // Compares two scores. - // Returns 0 if scores are equal, 1 if score2 is larger than score1, - // -1 otherwise. - // Note: this ordering causes Array.sort() to sort from highest to lowest score. - function scoreCompare(score1, score2) { - if (score1 === score2) { - return 0; - } - var comp = false; - if (score1 === null || score2 === null) { - comp = (score1 === null); - } else if (score1 === "dsq" || score2 === "dsq") { - comp = (score1 === "dsq"); - } else if (score1 === "dnc" || score2 === "dnc") { - comp = (score1 === "dnc"); - } else if (typeof score1 === "number" && typeof score2 === "number") { - comp = score1 < score2; - } else { - throw new TypeError("cannot compare scores '" + score1 + "' and '" + score2 + '"'); - } - return comp ? 1 : -1; - } - - // Compares two scores-arrays. - // Returns 0 if arrays are equal, 1 is scores2 is larger than scores1, - // -1 otherwise. - // Note: this ordering causes Array.sort() to sort from highest to lowest score. - function scoresCompare(scores1, scores2) { - var result = 0; - var i; - if (scores1.length !== scores2.length) { - throw new RangeError("cannot compare score arrays with different number of rounds"); - } - for (i = 0; i < scores1.length; i++) { - result = scoreCompare(scores1[i], scores2[i]); - if (result !== 0) - break; - } - return result; - } - - // Compare two 'team entries' (members of a scoreboard stage). - // 1 is teamEntry2 has higher scores than teamEntry1, or -if scores are - // equal- teamEntry1 has a higher team number. Returns -1 otherwise. - // Note: this ordering causes Array.sort() to sort from highest to lowest score, - // or in ascending team-id order. - function entryCompare(teamEntry1, teamEntry2) { - var result = scoresCompare(teamEntry1.sortedScores, teamEntry2.sortedScores); - if (result === 0) { - // Equal scores, ensure stable sort by introducing - // extra criterion. - // Note: team number's might be strings, so don't assume numeric - // compare is possible. - result = (teamEntry1.teamNumber > teamEntry2.teamNumber) ? 1 : -1; - } - return result; - } - - function createSortedScores(teamEntry) { - teamEntry.sortedScores = teamEntry.scores.slice(0); // create a copy - teamEntry.sortedScores.sort(scoreCompare); - teamEntry.highest = teamEntry.sortedScores[0]; - } - - function calculateRank(state,teamEntry) { - if (state.lastScores === null || scoresCompare(state.lastScores, teamEntry.sortedScores) !== 0) { - state.rank++; - } - state.lastScores = teamEntry.sortedScores; - teamEntry.rank = state.rank; - return state; - } - - // Sort by scores and compute rankings - for (var stageId in board) { - if (!board.hasOwnProperty(stageId)) { - continue; - } - var stage = board[stageId]; - - // Create sorted scores and compute highest score per team - stage.forEach(createSortedScores); - - // Sort teams based on sorted scores - stage.sort(entryCompare); - - // Compute ranking, assigning equal rank to equal scores - stage.reduce(calculateRank,{ - rank: 0, - lastScores: null - }); - } + var board = ranking.calculateScoreboard(this.scores, stageFilter); // Convert team number into team object for (var stageId in board) { From 6741d0406bf3277169c0364c87348d2913984af5 Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Mon, 30 Oct 2017 11:07:42 +0100 Subject: [PATCH 5/7] common/ranking: Refactor isValidScore out into its own common/scoring. --- spec/common/rankingSpec.js | 26 +------------------------- spec/common/scoringSpec.js | 30 ++++++++++++++++++++++++++++++ src/js/common/ranking.js | 20 ++++---------------- src/js/common/scoring.js | 27 +++++++++++++++++++++++++++ src/js/services/ng-scores.js | 5 +++-- 5 files changed, 65 insertions(+), 43 deletions(-) create mode 100644 spec/common/scoringSpec.js create mode 100644 src/js/common/scoring.js diff --git a/spec/common/rankingSpec.js b/spec/common/rankingSpec.js index b6343862..5c144be0 100644 --- a/spec/common/rankingSpec.js +++ b/spec/common/rankingSpec.js @@ -1,33 +1,9 @@ -describe('ranking', function () { +describe('common/ranking', function () { var ranking; beforeEach(function () { ranking = factory('common/ranking'); }); - describe('isValidScore', function () { - it('should accept valid scores', function () { - expect(ranking.isValidScore(0)).toBe(true); - expect(ranking.isValidScore(-1)).toBe(true); - expect(ranking.isValidScore(1000)).toBe(true); - expect(ranking.isValidScore("dnc")).toBe(true); // Did Not Compete - expect(ranking.isValidScore("dsq")).toBe(true); // DiSQualified - }); - - it('should reject invalid scores', function () { - expect(ranking.isValidScore(undefined)).toBe(false); - expect(ranking.isValidScore(null)).toBe(false); - expect(ranking.isValidScore(NaN)).toBe(false); - expect(ranking.isValidScore(Infinity)).toBe(false); - expect(ranking.isValidScore(-Infinity)).toBe(false); - expect(ranking.isValidScore("dnq")).toBe(false); - expect(ranking.isValidScore("foo")).toBe(false); - expect(ranking.isValidScore(true)).toBe(false); - expect(ranking.isValidScore(false)).toBe(false); - expect(ranking.isValidScore({})).toBe(false); - expect(ranking.isValidScore([])).toBe(false); - }); - }); - describe('calculateScoreboard', function () { it('should output used stages', function () { var board = ranking.calculateScoreboard([], { test: 1 }); diff --git a/spec/common/scoringSpec.js b/spec/common/scoringSpec.js new file mode 100644 index 00000000..7ceee024 --- /dev/null +++ b/spec/common/scoringSpec.js @@ -0,0 +1,30 @@ +describe('common/scoring', function () { + var scoring; + beforeEach(function () { + scoring = factory('common/scoring'); + }); + + describe('isValidScore', function () { + it('should accept valid scores', function () { + expect(scoring.isValidScore(0)).toBe(true); + expect(scoring.isValidScore(-1)).toBe(true); + expect(scoring.isValidScore(1000)).toBe(true); + expect(scoring.isValidScore("dnc")).toBe(true); // Did Not Compete + expect(scoring.isValidScore("dsq")).toBe(true); // DiSQualified + }); + + it('should reject invalid scores', function () { + expect(scoring.isValidScore(undefined)).toBe(false); + expect(scoring.isValidScore(null)).toBe(false); + expect(scoring.isValidScore(NaN)).toBe(false); + expect(scoring.isValidScore(Infinity)).toBe(false); + expect(scoring.isValidScore(-Infinity)).toBe(false); + expect(scoring.isValidScore("dnq")).toBe(false); + expect(scoring.isValidScore("foo")).toBe(false); + expect(scoring.isValidScore(true)).toBe(false); + expect(scoring.isValidScore(false)).toBe(false); + expect(scoring.isValidScore({})).toBe(false); + expect(scoring.isValidScore([])).toBe(false); + }); + }); +}); diff --git a/src/js/common/ranking.js b/src/js/common/ranking.js index e4ee86da..11b105b0 100644 --- a/src/js/common/ranking.js +++ b/src/js/common/ranking.js @@ -10,18 +10,7 @@ if (typeof exports === 'object' && typeof exports.nodeName !== 'string' && typeo } define("common/ranking", function (require, exports, module) { - /** - * Determine whether given score is valid, i.e. a number, or "dnc" (Did Not Compete), - * or "dsq" (DiSQualified). - * Note: `null` and `undefined` are invalid: remove the score to denote this instead. - * - * @param score {any} Score value to test - * @return true when score is valid - */ - function isValidScore(score) { - return typeof score === "number" && score > -Infinity && score < Infinity || - score === "dnc" || score === "dsq"; - } + var scoring = require("./scoring"); /** * Compute rankings based on given scores and a stageFilter. @@ -49,11 +38,11 @@ define("common/ranking", function (require, exports, module) { * @property teamNumber {number} * @property scores {Array.} * @property sortedScores {Array.} - * @property entries {Array.} + * @property entries {Array.} * @property highestScore {number} * * @param scores {Array.} Scores - * @param stageFilter {StageFilter} Hash of stageId => numberOfRounds + * @param stageFilter {{ [stageId: string]: number }} Hash of stageId => numberOfRounds * @return {Array.} Ranking based on filtered scores */ function calculateScoreboard(scores, stageFilter) { @@ -68,7 +57,7 @@ define("common/ranking", function (require, exports, module) { } // Ignore completely invalid scores - if (!isValidScore(s.score)) { + if (!scoring.isValidScore(s.score)) { return false; } @@ -216,6 +205,5 @@ define("common/ranking", function (require, exports, module) { return board; } - exports.isValidScore = isValidScore; exports.calculateScoreboard = calculateScoreboard; }); diff --git a/src/js/common/scoring.js b/src/js/common/scoring.js new file mode 100644 index 00000000..e6f0b001 --- /dev/null +++ b/src/js/common/scoring.js @@ -0,0 +1,27 @@ +/** + * Ranking calculation used on server and client. + */ + +// UMD boilerplate (modified from https://github.com/umdjs/umd/blob/master/templates/commonjsAdapter.js) +if (typeof exports === 'object' && typeof exports.nodeName !== 'string' && typeof define !== 'function') { + var define = function (name, factory) { + factory(require, exports, module); + }; +} +define("common/scoring", function (require, exports, module) { + + /** + * Determine whether given score is valid, i.e. a number, or "dnc" (Did Not Compete), + * or "dsq" (DiSQualified). + * Note: `null` and `undefined` are invalid: remove the score to denote this instead. + * + * @param score {any} Score value to test + * @return true when score is valid + */ + function isValidScore(score) { + return typeof score === "number" && score > -Infinity && score < Infinity || + score === "dnc" || score === "dsq"; + } + + exports.isValidScore = isValidScore; +}); diff --git a/src/js/services/ng-scores.js b/src/js/services/ng-scores.js index d0c37e6c..6662af5a 100644 --- a/src/js/services/ng-scores.js +++ b/src/js/services/ng-scores.js @@ -5,12 +5,13 @@ define('services/ng-scores',[ 'services/ng-services', 'services/log', + 'common/scoring', 'common/ranking', 'services/ng-fs', 'services/ng-stages', 'factories/poller', 'services/ng-teams', -],function(module, log, ranking) { +],function(module, log, scoring, ranking) { "use strict"; // Current file version for scores. @@ -537,7 +538,7 @@ define('services/ng-scores',[ // mean that one could 'reset' a team's score for that round. // If a team did not play in a round, there will simply be no // entry in scores. - if (!ranking.isValidScore(s.score)) { + if (!scoring.isValidScore(s.score)) { s.error = new InvalidScoreError(s.score); return; } From 6fd73bf00e8c232f70926f92e9858645a915e113 Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Wed, 25 Oct 2017 21:48:00 +0200 Subject: [PATCH 6/7] ranking: Prevent scores with round <= 0 from ending up in rankings. --- spec/common/rankingSpec.js | 2 +- spec/services/ng-scoresSpec.js | 2 +- src/js/common/ranking.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/common/rankingSpec.js b/spec/common/rankingSpec.js index 5c144be0..01cc73a9 100644 --- a/spec/common/rankingSpec.js +++ b/spec/common/rankingSpec.js @@ -105,7 +105,7 @@ describe('common/ranking', function () { { teamNumber: 41, stageId: 'test', round: 0, score: 0, published: true }, { teamNumber: 41, stageId: 'test', round: 4, score: 0, published: true }, ], { test: 3 }); - expect(board['test'].length).toEqual(1); + expect(board['test'].length).toEqual(0); }); it("should ignore invalid scores", function () { diff --git a/spec/services/ng-scoresSpec.js b/spec/services/ng-scoresSpec.js index ddb72b16..8c5f8e45 100644 --- a/spec/services/ng-scoresSpec.js +++ b/spec/services/ng-scoresSpec.js @@ -373,7 +373,7 @@ describe('ng-scores',function() { expect($scores.scores[0].error).toEqual(jasmine.any($scores.UnknownStageError)); expect($scores.scores[1].error).toEqual(jasmine.any($scores.UnknownRoundError)); expect($scores.scores[2].error).toEqual(jasmine.any($scores.UnknownRoundError)); - expect(board["test"].length).toEqual(1); + expect(board["test"].length).toEqual(0); expect($scores.validationErrors.length).toEqual(3); }); diff --git a/src/js/common/ranking.js b/src/js/common/ranking.js index 11b105b0..b1321f1a 100644 --- a/src/js/common/ranking.js +++ b/src/js/common/ranking.js @@ -62,7 +62,7 @@ define("common/ranking", function (require, exports, module) { } // Ignore score if filtered - if (!stageFilter[s.stageId] || s.round > stageFilter[s.stageId]) { + if (!stageFilter[s.stageId] || s.round > stageFilter[s.stageId] || s.round < 1) { return false; } From 3c26de430a01ac9a7b38d9230e102dd778ef50d1 Mon Sep 17 00:00:00 2001 From: Martin Poelstra Date: Wed, 25 Oct 2017 22:12:02 +0200 Subject: [PATCH 7/7] views/ranking: Prevent 'crash' in CSV export when score for unknown team is submitted. --- src/js/views/ranking.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/js/views/ranking.js b/src/js/views/ranking.js index 3cb5c7b1..a3b186dd 100644 --- a/src/js/views/ranking.js +++ b/src/js/views/ranking.js @@ -134,8 +134,8 @@ define('views/ranking',[ var rows = ranking.map(function(entry) { return [ entry.rank, - entry.team.number, - entry.team.name, + entry.team ? entry.team.number : 0, + entry.team ? entry.team.name : "", entry.highest, ].concat(entry.scores); });