Fix magic numbers

otacke · Aug 6, 2024 · d1c9a70 · d1c9a70
1 parent 1bb3cfb
commit d1c9a70
Show file tree

Hide file tree

Showing 8 changed files with 60 additions and 37 deletions.
diff --git a/src/scripts/h5p-crossword-cell.js b/src/scripts/h5p-crossword-cell.js
@@ -368,7 +368,7 @@ export default class CrosswordCell {
     this.cell.classList.remove('h5p-crossword-solution-neutral');
 
     if (state) {
-      const className = 'h5p-crossword-solution' + ((state) ? `-${state}` : '');
+      const className = `h5p-crossword-solution${  (state) ? `-${state}` : ''}`;
       this.cell.classList.add(className);
     }
   }
@@ -435,7 +435,7 @@ export default class CrosswordCell {
       return;
     }
 
-    const className = 'h5p-crossword-highlight' + ((type) ? `-${type}` : '');
+    const className = `h5p-crossword-highlight${  (type) ? `-${type}` : ''}`;
     this.cell.classList.add(className);
   }
 
@@ -449,7 +449,7 @@ export default class CrosswordCell {
       this.cell.classList.remove('h5p-crossword-highlight-focus');
     }
     else {
-      const className = 'h5p-crossword-highlight' + ((type) ? `-${type}` : '');
+      const className = `h5p-crossword-highlight${  (type) ? `-${type}` : ''}`;
       this.cell.classList.remove(className);
     }
   }

diff --git a/src/scripts/h5p-crossword-content.js b/src/scripts/h5p-crossword-content.js
@@ -4,6 +4,12 @@ import CrosswordTable from '@scripts/h5p-crossword-table';
 import CrosswordSolutionWord from '@scripts/h5p-crossword-solution-word';
 import CrosswordGenerator from '@scripts/h5p-crossword-generator';
 
+/** @constant {number} MIN_WORDS_FOR_CROSSWORD Minimum number of words for crossword. */
+const MIN_WORDS_FOR_CROSSWORD = 2;
+
+/** @constant {number} MAXIMUM_TRIES Maximum number of tries to generate crossword grid */
+const MAXIMUM_TRIES = 20;
+
 /** Class representing the content */
 export default class CrosswordContent {
   /**
@@ -35,7 +41,7 @@ export default class CrosswordContent {
       let crosswordGenerator;
       let grid;
 
-      if (params.words.length < 2) {
+      if (params.words.length < MIN_WORDS_FOR_CROSSWORD) {
         errorMessages.push(params.l10n.couldNotGenerateCrosswordTooFewWords);
       }
       else {
@@ -45,7 +51,7 @@ export default class CrosswordContent {
             poolSize: params.poolSize
           }
         });
-        grid = crosswordGenerator.getSquareGrid(CrosswordContent.MAXIMUM_TRIES);
+        grid = crosswordGenerator.getSquareGrid(MAXIMUM_TRIES);
 
         if (!grid) {
           errorMessages.push(params.l10n.couldNotGenerateCrossword);
@@ -228,7 +234,7 @@ export default class CrosswordContent {
    * @param {boolean} [params.keepCorrectAnswers] If true, correct answers are kept.
    */
   reset(params = {}) {
-    if (this.params.words.length < 2) {
+    if (this.params.words.length < MIN_WORDS_FOR_CROSSWORD) {
       return;
     }
 
@@ -261,7 +267,7 @@ export default class CrosswordContent {
    * @returns {number} Score.
    */
   getScore() {
-    if (this.params.words.length < 2) {
+    if (this.params.words.length < MIN_WORDS_FOR_CROSSWORD) {
       return 0;
     }
 
@@ -273,7 +279,7 @@ export default class CrosswordContent {
    * @returns {number} Maximum score.
    */
   getMaxScore() {
-    if (this.params.words.length < 2) {
+    if (this.params.words.length < MIN_WORDS_FOR_CROSSWORD) {
       return 0;
     }
 
@@ -285,7 +291,7 @@ export default class CrosswordContent {
    * @returns {object|undefined} Current state.
    */
   getCurrentState() {
-    if (this.params.words.length < 2 || !this.table) {
+    if (this.params.words.length < MIN_WORDS_FOR_CROSSWORD || !this.table) {
       return;
     }
 
@@ -339,7 +345,7 @@ export default class CrosswordContent {
    * Show solution.
    */
   showSolutions() {
-    if (this.params.words.length < 2) {
+    if (this.params.words.length < MIN_WORDS_FOR_CROSSWORD) {
       return;
     }
 
@@ -475,6 +481,3 @@ export default class CrosswordContent {
     document.querySelector('head').appendChild(style);
   }
 }
-
-/** @constant {number} Maximum number of tries to generate crossword grid */
-CrosswordContent.MAXIMUM_TRIES = 20;
diff --git a/src/scripts/h5p-crossword-generator.js b/src/scripts/h5p-crossword-generator.js
@@ -70,7 +70,7 @@ export default class CrosswordGenerator {
     let ratioBest = 0;
 
     for (let i = 0; i < triesMax; i++) {
-      const gridCurrent = this.getGrid(10);
+      const gridCurrent = this.getGrid();
       if (gridCurrent === null) {
         continue; // Could not create grid
       }
@@ -132,16 +132,16 @@ export default class CrosswordGenerator {
       }
       else {
         // Place first answer in the middle of the grid
-        let row = Math.floor(this.cells.length / 2);
-        let column = Math.floor(this.cells[0].length / 2);
+        let row = Math.floor(this.cells.length / 2); // eslint-disable-line no-magic-numbers
+        let column = Math.floor(this.cells[0].length / 2); // eslint-disable-line no-magic-numbers
         const wordElement = this.wordElements[0];
 
         const startOrientation = this.getRandomOrientation();
         if (startOrientation === 'across') {
-          column -= Math.floor(wordElement.answer.length / 2);
+          column -= Math.floor(wordElement.answer.length / 2); // eslint-disable-line no-magic-numbers
         }
         else {
-          row -= Math.floor(wordElement.answer.length / 2);
+          row -= Math.floor(wordElement.answer.length / 2); // eslint-disable-line no-magic-numbers
         }
 
         if (this.canPlaceAnswerAt(wordElement.answer, { row: row, column: column, orientation: startOrientation }) !== false) {
@@ -224,7 +224,9 @@ export default class CrosswordGenerator {
    * @returns {string} Random orientation
    */
   getRandomOrientation() {
-    return Math.floor(Math.random() * 2) ? 'across' : 'down';
+    const ORIENTATION_COUNT = 2;
+
+    return Math.floor(Math.random() * ORIENTATION_COUNT) ? 'across' : 'down';
   }
 
   /**
@@ -340,7 +342,7 @@ export default class CrosswordGenerator {
     if (this.cells[position.row][position.column] === null) {
       return 0; // no intersection
     }
-    if (this.cells[position.row][position.column]['char'] === char) {
+    if (this.cells[position.row][position.column].char === char) {
       return 1; // intersection!
     }
 
@@ -393,7 +395,7 @@ export default class CrosswordGenerator {
       // the character below it intersects with the current word
       for (let row = position.row - 1, column = position.column, i = 0; row >= 0 && column < position.column + answer.length; column++, i++) {
         const isEmpty = (this.cells[row][column] === null);
-        const isIntersection = this.cells[position.row][column] !== null && this.cells[position.row][column]['char'] === answer.charAt(i);
+        const isIntersection = this.cells[position.row][column] !== null && this.cells[position.row][column].char === answer.charAt(i);
         if (!isEmpty && !isIntersection) {
           return false;
         }
@@ -402,7 +404,7 @@ export default class CrosswordGenerator {
       // same deal as above, we just search in the row below the word
       for (let r = position.row + 1, c = position.column, i = 0; r < this.cells.length && c < position.column + answer.length; c++, i++) {
         const isEmpty = (this.cells[r][c] === null);
-        const isIntersection = this.cells[position.row][c] !== null && this.cells[position.row][c]['char'] === answer.charAt(i);
+        const isIntersection = this.cells[position.row][c] !== null && this.cells[position.row][c].char === answer.charAt(i);
         if (!isEmpty && !isIntersection) {
           return false;
         }
@@ -450,7 +452,7 @@ export default class CrosswordGenerator {
       // current word
       for (let column = position.column - 1, row = position.row, i = 0; column >= 0 && row < position.row + answer.length; row++, i++) {
         const isEmpty = this.cells[row][column] === null;
-        const isIntersection = this.cells[row][position.column] !== null && this.cells[row][position.column]['char'] === answer.charAt(i);
+        const isIntersection = this.cells[row][position.column] !== null && this.cells[row][position.column].char === answer.charAt(i);
         const can_place_here = isEmpty || isIntersection;
         if (!can_place_here) {
           return false;
@@ -460,7 +462,7 @@ export default class CrosswordGenerator {
       // same deal, but look at the column to the right
       for (let column = position.column + 1, row = position.row, i = 0; row < position.row + answer.length && column < this.cells[row].length; row++, i++) {
         const isEmpty = this.cells[row][column] === null;
-        const isIntersection = this.cells[row][position.column] !== null && this.cells[row][position.column]['char'] === answer.charAt(i);
+        const isIntersection = this.cells[row][position.column] !== null && this.cells[row][position.column].char === answer.charAt(i);
         const can_place_here = isEmpty || isIntersection;
         if (!can_place_here) {
           return false;
@@ -500,8 +502,8 @@ export default class CrosswordGenerator {
 
       for (let j = 0; j < possibleLocations.length; j++) {
         const point = possibleLocations[j];
-        const row = point['row'];
-        const column = point['column'];
+        const row = point.row;
+        const column = point.column;
         // the c - i, and r - i here compensate for the offset of character in the answer
         const intersectionsAcross = this.canPlaceAnswerAt(answer, { row: row, column: column - i, orientation: 'across' });
         const intersectionsDown = this.canPlaceAnswerAt(answer, { row: row - i, column: column, orientation: 'down' });
@@ -555,11 +557,13 @@ export default class CrosswordGenerator {
    * @returns {object} Word item.
    */
   createWordElements(words, poolSize) {
+    const MIN_POOL_SIZE = 2; // Minimum number of words to keep in pool
+
     if (typeof poolSize !== 'number' || poolSize === 0) {
       poolSize = null;
     }
     else {
-      poolSize = Math.max(2, poolSize);
+      poolSize = Math.max(MIN_POOL_SIZE, poolSize);
     }
 
     // Add index to word element

diff --git a/src/scripts/h5p-crossword-input.js b/src/scripts/h5p-crossword-input.js
@@ -2,6 +2,12 @@ import Overlay from '@scripts/h5p-crossword-overlay';
 import Util from '@services/util';
 import CrosswordCharList from '@scripts/h5p-crossword-char-list';
 
+/** @constant {number} ANDROID_KEYCODE_229 Android specific keycode */
+const ANDROID_KEYCODE_229 = 229;
+
+/** @constant {number} DEFAULT_DOM_RENDER_TIMEOUT_MS Default timeout for DOM rendering */
+const DEFAULT_DOM_RENDER_TIMEOUT_MS = 25;
+
 /** Class representing the content */
 export default class CrosswordInput {
   /**
@@ -232,12 +238,12 @@ export default class CrosswordInput {
             text: after,
             readOffset: -1 // don't read
           });
-        }, 25); // selectionStart will be 0 before DOM rendered
+        }, DEFAULT_DOM_RENDER_TIMEOUT_MS); // selectionStart will be 0 before DOM rendered
       }, false);
 
       // Only update table if input is valid or using arrow keys
       inputField.addEventListener('keyup', (event) => {
-        if (event.keyCode === 229) {
+        if (event.keyCode === ANDROID_KEYCODE_229) {
           return; // workaround for Android specific code, no event.key equivalent
         }
 

diff --git a/src/scripts/h5p-crossword-solution-word.js b/src/scripts/h5p-crossword-solution-word.js
@@ -1,5 +1,6 @@
 import CrosswordCell from '@scripts/h5p-crossword-cell';
 import Util from '@services/util';
+import { CELL_FONT_SIZE_DIVIDER } from '@scripts/h5p-crossword-table.js';
 
 /** Class representing the content */
 export default class CrosswordSolutionWord {
@@ -57,7 +58,7 @@ export default class CrosswordSolutionWord {
       .split('')
       .forEach((character, index) => {
         cells[index] = new CrosswordCell({
-          width: 100 / solutionWord.length,
+          width: 100 / solutionWord.length, // eslint-disable-line no-magic-numbers
           solution: solutionWord[index],
           clueIdMarker: index + 1 // Technically, it's a solution marker ...
         });
@@ -109,7 +110,7 @@ export default class CrosswordSolutionWord {
     // Using table border would yield 1 pixel gaps sometimes, hmm
 
     // Magic number found by testing
-    this.content.style.fontSize = `${cellWidth / 2}px`;
+    this.content.style.fontSize = `${cellWidth / CELL_FONT_SIZE_DIVIDER}px`;
     this.cells.forEach((cell) => {
       cell.setWidth(cellWidth);
     });

diff --git a/src/scripts/h5p-crossword-table.js b/src/scripts/h5p-crossword-table.js
@@ -1,6 +1,9 @@
 import CrosswordCell from '@scripts/h5p-crossword-cell';
 import Util from '@services/util';
 
+/** @constant {number} CELL_FONT_SIZE_DIVIDER Divisor found by testing */
+export const CELL_FONT_SIZE_DIVIDER = 2;
+
 /** Class representing the content */
 export default class CrosswordTable {
   /**
@@ -131,7 +134,9 @@ export default class CrosswordTable {
           else {
             this.moveTo({
               row: parseInt(target.dataset.row),
-              column: document.querySelector('[data-row="' + target.dataset.row + '"]:last-of-type').dataset.col
+              column: document.querySelector(
+                `[data-row="${target.dataset.row}"]:last-of-type`
+              ).dataset.col
             });
           }
           break;
@@ -234,7 +239,7 @@ export default class CrosswordTable {
         solution: param.solution,
         solutionIndex: param.solutionIndex,
         solutionLength: param.solutionLength,
-        width: 100 / dimensions.columns,
+        width: 100 / dimensions.columns, // eslint-disable-line no-magic-numbers
         clueIdMarker: param.clueIdMarker,
         clue: param.clue,
         clueIdAcross: param.clueIdAcross,
@@ -637,8 +642,7 @@ export default class CrosswordTable {
     // Didn't work well by just using CSS
     const cellWidth = this.content.clientWidth / this.params.dimensions.columns;
 
-    // Magic number found by testing
-    this.content.style.fontSize = `${cellWidth / 2}px`;
+    this.content.style.fontSize = `${cellWidth / CELL_FONT_SIZE_DIVIDER}px`;
 
     return this.content.getBoundingClientRect();
   }

diff --git a/src/scripts/h5p-crossword.js b/src/scripts/h5p-crossword.js
@@ -2,6 +2,9 @@
 import CrosswordContent from '@scripts/h5p-crossword-content';
 import Util from '@services/util';
 
+/** @constant {number} DOM_REGISTER_DELAY_MS Delay before resizing after DOM registered. */
+const DOM_REGISTER_DELAY_MS = 100;
+
 /**
  * Class for H5P Crossword.
  */
@@ -191,7 +194,7 @@ export default class Crossword extends H5P.Question {
     Util.waitForDOM('.h5p-crossword-input-container', () => {
       setTimeout(() => {
         this.trigger('resize');
-      }, 100);
+      }, DOM_REGISTER_DELAY_MS);
     });
   }
 

diff --git a/src/scripts/services/util.js b/src/scripts/services/util.js
@@ -160,13 +160,15 @@ class Util {
    * @param {number} [interval] Time interval in ms to check for element.
    */
   static waitForDOM(selector, success, error = (() => {}), tries = 50, interval = 100) {
+    const INTERVAL_MIN_MS = 50;
+
     if (tries === 0 || !selector || typeof success !== 'function' || typeof error !== 'function') {
       error();
       return;
     }
 
     // Try to keep sensible
-    interval = Math.max(interval, 50);
+    interval = Math.max(INTERVAL_MIN_MS, interval);
 
     const content = document.querySelector(selector);
     if (!content) {