4
\$\begingroup\$

My idea is to have objects which contain random numbers. The random numbers are keeped in some data structure (within the objects). Furthermore the objects shall have methods for working with the random numbers.

var oo = oo || {}; // 'oo' serves as a namespace. Have choosen these chars just because they are easy to type.

oo.randomNumbers; 
oo.alignedTextCreator;

oo.RandomNumbers = 
  function(quantityRandomNumbers, inclusiveMin, exclusiveMax) {
  // ------- Validations --------------------------------------
  if (inclusiveMin >= exclusiveMax) {
    throw new Error('exclusiveMax have to larger then inclusiveMax.');
  }

  if (!oo.RandomNumbers.validateNumber(quantityRandomNumbers, 1, 10000)) {
    throw new Error(
      'Assigned parameter quantityRandomNumbers is not valid.');
  }

  if (!oo.RandomNumbers.validateNumber(inclusiveMin, 0, 100000)) {
    throw new Error('Assigned parameter inclusiveMin is not valid.');
  }

  if (!oo.RandomNumbers.validateNumber(exclusiveMax, 0, 100000)) {
    throw new Error('Assigned parameter exclusiveMin is not valid.');
  }
  
  // ------- Property declarations --------------------------------
  this.randomNumbers = [];
  this.min = exclusiveMax - 1; // Using the min-, max-search algorithm.
  this.max = inclusiveMin;
  
  // ------- Methods ----------------------------------------------
  
  // Can be used to print a CSS-styled number (from within the 
  //  randomNumbers-array) to the screen.
  this.consoleLogNumberStyled = function(text, color, val, fontFamily) {
    text        = text || '%c%s';
    color       = color || 'black';
    fontFamily  = fontFamily || 'courier';

    console.log(text,
                'color: ' + color + '; font-family: ' + fontFamily + ';',
                val );
  }
  // -----------------------------------------------------------
  
  // Create random numbers and push them to the array. 
  //  Keep record about the smallest and largest number.
  for (var i= 0; i < quantityRandomNumbers; i++) {
    this.randomNumbers.push(
      Math.floor(Math.random() * (exclusiveMax - inclusiveMin))
       + inclusiveMin);

    if (this.randomNumbers[i] < this.min) {
      this.min = this.randomNumbers[i];
    }

    if (this.randomNumbers[i] > this.max) {
      this.max = this.randomNumbers[i];
    } 
  }
}

// Static method. Used the RandomNumbers function self to validate the
//  given parameter.
oo.RandomNumbers.validateNumber = function(numberToValidate, minLength, maxLength) {
  minLength = minLength || 0;
  maxLength = maxLength || 1000000;

  if (typeof numberToValidate != 'number' || numberToValidate === NaN 
    || numberToValidate < minLength || numberToValidate > maxLength) {
    return false;
  } else {
    return true;
  }
}

// For accomplishing an aligned output on the console.
oo.AlignedTextCreator = function(columnWidthNeeded) {
  if (!oo.RandomNumbers.validateNumber(columnWidthNeeded, 1, 10000)) {
    throw new Error(
      'Assigned parameter columnWidthNeeded is not valid.');
  }

  this.columnWidthNeeded = new Array(columnWidthNeeded).join(' ');

  this.createAlignedText = function(i, text) {
    return (this.columnWidthNeeded + i).slice(-this.columnWidthNeeded.length)
      + ' : ' + text;
  }
}

// -- Create & use a RandomNumbers object. --------------------------------------------
oo.randomNumbers = new oo.RandomNumbers(1000, 10, 100000);
oo.alignedTextCreator = new oo.AlignedTextCreator((oo.quantityRandomNumbers + '').length);
// Print every number within the set to the console.
for (oo.i = 0; oo.i < 1000; oo.i++) {
  console.log( 
    oo.alignedTextCreator
      .createAlignedText(oo.i,
                         oo.randomNumbers.randomNumbers[oo.i])
  );
}
// Print min and max of the created set to the console.
oo.randomNumbers.consoleLogNumberStyled(
  'Min: %c%s',
  'magenta',
  oo.randomNumbers.min);
oo.randomNumbers.consoleLogNumberStyled(
  'Max: %c%s',
  'green',
  oo.randomNumbers.max);

Before I go on with adding more methods (like for example "shuffle") I would appreciate your review. Especially concerning:

  • Is my parameter validation done correctly and sufficient?

  • Design: I have added the "validateNumber" method to the constructor function itself as a static method. "consoleLogNumberStyled" I have made an object method because it would make no sense without having an object (with random numbers).

Are these decisions correct? Or are there good reason for changing it somehow?

All other hints and comments how to improve the function are appreciated too.

\$\endgroup\$
  • \$\begingroup\$ "exclusiveMax have to larger then inclusiveMax" I think you mean inclusiveMin? \$\endgroup\$ – Feathercrown Feb 20 '17 at 0:55
  • \$\begingroup\$ Yep. Indeed. I have got a typo there. Many thanks for your hint. :) \$\endgroup\$ – michael.zech Feb 20 '17 at 3:02
  • \$\begingroup\$ You're welcome, always glad to help. :) \$\endgroup\$ – Feathercrown Feb 20 '17 at 20:02
1
\$\begingroup\$

Naming:

Instead of

oo.RandomNumbers = function(quantityRandomNumbers, inclusiveMin, exclusiveMax) { ... }

supply comments to document your code and simplify to

// Creates random numbers from min (inclusive) to max (exclusive):
oo.RandomNumbers = function(length, min, max) { ... }

You might want to follow an established format such as JSDoc.

Instead of

this.min = exclusiveMax - 1;
this.max = inclusiveMin;

chose more descriptive name to avoid mix-ups, e.g.

this.lowest = exclusiveMax - 1;
this.highest = inclusiveMin;

Also,

oo.randomNumbers.randomNumbers[oo.i]

is a duplication. Go with something simpler such as

oo.randomNumbers.values[oo.i]

Error handling:

Instead of throwing a generic Error for

if (inclusiveMin >= exclusiveMax) {
  throw new Error('exclusiveMax have to larger then inclusiveMax.');
}

throw a more specific RangeError.

Get rid of all kinds of arbitrary input validations such as

if (!oo.RandomNumbers.validateNumber(quantityRandomNumbers, 1, 10000)) {
  throw new Error(
    'Assigned parameter quantityRandomNumbers is not valid.');
}

if (!oo.RandomNumbers.validateNumber(inclusiveMin, 0, 100000)) {
  throw new Error('Assigned parameter inclusiveMin is not valid.');
}

if (!oo.RandomNumbers.validateNumber(exclusiveMax, 0, 100000)) {
  throw new Error('Assigned parameter exclusiveMin is not valid.');
}

and simply write robust code that handles all Number inputs in a consistent manner:

  • The maximum array length is 2³² - 1 and everything above that already causes a RangeError to be thrown.

  • The lower and upper limit for min and max could more sensibly be set to Number.MIN_SAFE_INTEGER and Number.MAX_SAFE_INTEGER or Number.MIN_VALUE and Number.MAX_VALUE or simply -Infinity and Infinity. Let the user of your module chose which limits are relevant instead of arbitrarily choosing 0 and 100000.

  • NaN values are handled in a robust and consistent manner already: if either the minimum or maximum is NaN, the whole output will be `NaN´, too.

Separate concerns:

oo.consoleLogNumberStyled()
oo.AlignedTextCreator()

don't belong on the same namespace or module as the random number generator as they are not related at all.

Namespace:

You use the oo object for unrelated things:

  1. to act as a module holding your methods and constructor functions
  2. to act as a block scope and thus prevent you from polluting the global scope

Just focus on 1. and for 2. use either an IIFE or a simple block scope.

Also, instead of

for (oo.i = 0; oo.i < 1000; oo.i++) { ...  }

use a local variable via var or even more localized via let

for (let i = 0; i < 1000; i++) { ... }

Overall complexity:

Instead of introducing a new class for something as simple as a random number array, write simple reusable helper functions which return arrays:

// Random number between min (inclusive) and max (exclusive):
function randomInteger(min, max) {
  if (min >= max) throw new RangeError("max must be larger than min");
  return Math.floor(Math.random() * (max - min)) + min;
}

// Array of 'length' random numbers between min (inclusive) and max (exclusive):
function randomIntegers(length, min, max) {
  return Array.from({length: length}, () => randomInteger(min, max));
}

let integers = randomIntegers(10, 0, 5);
let min = Math.min(...integers);
let max = Math.max(...integers);

console.log(integers, min, max);

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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