How to improve this js function so as to make it faster and also better unit-testable?

Question

I have a simple program:

function counterPattern() {
  for (let i = 1; i <= 100; i++) {
    if (i % 3 === 0 && i % 5 === 0) {
      console.log("c");
    } else if (i % 3 === 0) {
      console.log("a");
    } else if (i % 5 === 0) {
      console.log("b");
    }
  }
}

counterPattern();

I was asked, in steps, to improve the above function:

Decrease the time of execution, and
Write unit-test(s) for it

This lead me to change the function to make it suitable for writing unit test(s).

Following is an attempt in that direction:

function counterPattern(){
    const pattern = []; // for making it unit-testable; returning pattern will help with snapshotting/matching without losing the order;
    for ( let i=1; i<=100 ; i++ ) {
        const rem3 = i % 3; // separate calculation to + the speed; same for next-line;
        const rem5 = i % 5;
        if ( rem3 === 0 && rem5 === 0 ) {
            pattern.push('c');
        }else if ( rem3 === 0 ){
            pattern.push('a');
        }else if ( rem5 === 0){
            pattern.push('b');
        }
    }
    // returning array would allow the caller to format the pattern
    return pattern; 
}

let result = counterPattern();
console.log(result);

My question

How can the original program be written differently:

so that it is faster in execution,
and such that it is unit-testable;

Do you have any comments on my attempt?

I don't think snapshots are necessary for testing this function directly. Since it looks like a pure function from what I can tell, the output will always be the same. I tend to follow KentCDodds testing paradigms though so it's your prerogative of course. — Jacob, Aug 14 '20 at 04:18
About faster execution: https://en.wikipedia.org/wiki/Fizz_buzz - it's an **extremely** well known problem, with probably every solution you could think of in every language there is being somewhere on the internet. — ASDFGerte, Aug 14 '20 at 04:20
@Jacob - thanks for your input; the snapshot-testing was on my mind for a use-case such that the function is changed in future - but the spec is same; or in reverse if we have a result to start with and function body is developing; also, will check the KentCDodds - it's new to me. — H S, Aug 14 '20 at 04:28

trincot · Answer 1 · 2020-08-14T06:07:02.220

1. Speed

As to the speed of the algorithm, there are the following considerations:

About half of the loop's iterations will lead to no output. This could be improved;
There is a pattern that repeats every 15 iterations, because the least common multiple of 3 and 5 is 15.

So you could hardcode that pattern into your code, and perform the output corresponding to the range of 1..15, and then repeat:

function counterPattern() {
  for (let i = 0; i < 90; i += 15) {
    console.log("a"); // 3 + i
    console.log("b"); // 5 + i
    console.log("a"); // 6 + i
    console.log("a"); // 9 + i
    console.log("b"); // 10 + i
    console.log("a"); // 12 + i
    console.log("c"); // 15 + i
  }
  // The remainder: values between 90 and 100:
  console.log("a"); // 93
  console.log("b"); // 95
  console.log("a"); // 96
  console.log("a"); // 99
  console.log("b"); // 100
}

counterPattern();

2. Testing

To make this code testable, you have several options. Using an array, like you did, is one of them. You could also consider turning the function into a generator, and then use yield to output the values.

But there is also a solution where you don't have to touch the function's code: Either use mocking or spying to tap into console.log. Unit testing libraries usually offer such features.

Secondly, there is no better reference to test against than the original function. So you could first run the original function to collect the expected output, and then run the improved implementation. Finally, the two outputs should be compared.

Here is how mocking would work without the use of an external library:

function test() { // Wrapper to keep the mocking local

  function orig_counterPattern() { // The reference implementation
    for (let i = 1; i <= 100; i++) {
      if (i % 3 === 0 && i % 5 === 0) {
        console.log("c");
      } else if (i % 3 === 0) {
        console.log("a");
      } else if (i % 5 === 0) {
        console.log("b");
      }
    }
  }

  function counterPattern() { // Our own implementation
    for (let i = 0; i < 90; i += 15) {
      console.log("a"); // 3 + i
      console.log("b"); // 5 + i
      console.log("a"); // 6 + i
      console.log("a"); // 9 + i
      console.log("b"); // 10 + i
      console.log("a"); // 12 + i
      console.log("c"); // 15 + i
    }
    // The remainder: values between 90 and 100:
    console.log("a"); // 93
    console.log("b"); // 95
    console.log("a"); // 96
    console.log("a"); // 99
    console.log("b"); // 100
  }

  // Mock console object:
  let console = {
    log: (value) => output.push(value)
  };

  // Collect the expected output
  let output = [];
  orig_counterPattern();
  let reference = [...output];

  // Run our own implementation
  output = [];
  counterPattern();
  let result = [...output];

  // Stop mocking the console object
  console = globalThis.console;
  // Compare
  console.assert(output.length === reference.length, "incorrect number of outputs");
  console.assert(reference.every((ref, i) => ref === result[i], "mismatch"));
}

test();
console.log("Test completed.");

Shorter code

First, you could of course hardcode the output completely, even avoiding the loop. But that would go against the principle that we should not repeat ourselves (DRY).

Here is a variant that is a bit more DRY:

function counterPattern() {
  let pattern = "abaabac".repeat(105/15).slice(0, -2);
  for (let c of pattern) console.log(c);
}

counterPattern();

How to improve this js function so as to make it faster and also better unit-testable?

My question

1 Answers1

1. Speed

2. Testing

Shorter code