Return to Revisions

Getting the length of a string

You split the string into characters and then you count the array's length. While this works, there's an even simpler way of doing it, using the length property of a String:

const length = msgDiv.value.length;

Changing getStrLen

You can cache the DOM elements. Then you don't need to re-query them, each time the function is called:

const strlenInput = document.getElementById('textbox1');
const strlenDisplay = document.getElementById('msg');
const getStrLen = () => {
 strlenDisplay.textContent = strlenInput.value.length;
}

Now this alone won't work in the head-element of your document, as both elements are not available yet. You can either:

• wait until the DOM is ready to run that script, like document.addEventListener('DOMContentLoaded', function(event) { [...] }, or
• place the Javascript at the end of your page instead

This is explained in detail here: $(document).ready(function(){}); vs script at the bottom of page.

Creating the Fibonacci sequence

genFibonacci() takes one parameter, so you pass an unused parameter, when calling the function recursively:

genFibonacci(times - 1, false);

There's a flaw in your logic: You don't reset the values each time the button is clicked. If the user's input is 1 and they hit the generate button 5 times, the result is not 0 but:

 0 1 1 2 3

With this in mind, let's try to change your algorithm, that it won't rely on external variables. We also don't necessarily need recursion here:

const getFibonacciValues = (n) => {
 const values = [];
 if (n >= 1) {
 values.push(0);
 }
 
 if (n >= 2) {
 values.push(1);
 }
 for (let i = 2; i < n; ++i) {
 values.push(values[i - 2] + values[i -1]);
 }
 return values;
}

Now we have a function, which purpose is solely to create the values. It doesn't have a clue, how you're going to output them. outFib() is doing this for us now:

const outFib = () => {
 const n = fibInput.value;
 const values = getFibonacciValues(n);
 fibDisplay.textContent = values.join(', ');
}

Now, this still has flaws, so we add a little bit of validation at least:

• use parseInt to parse the value
• check whether the input is at least 1
• check whether the input is a number

The final result could look like this:

const fibButton = document.getElementById('fibClick');
const fibInput = document.getElementById('fibText');
const fibDisplay = document.getElementById('fibseq');
const getFibonacciValues = (n) => {
 const values = [];
 if (n >= 1) {
 values.push(0);
 }
 
 if (n >= 2) {
 values.push(1);
 }
 
 for (let i = 2; i < n; ++i) {
 values.push(values[i - 2] + values[i -1]);
 }
 return values;
};
const outFib = () => {
 const n = parseInt(fibInput.value);
 if (n < 1 || isNaN(n)) {
 fibDisplay.textContent = 'Please insert a number greater than 0';
 return;
 }
 
 const values = getFibonacciValues(n);
 fibDisplay.textContent = values.join(', ');
};
fibClick.addEventListener('click', outFib);

<input type="text" id="fibText">
<button id="fibClick">Generate Fibonacci Sequence</button>
<div id="fibseq"></div>

Further improvements

This will fail for large inputs – try 2000 for example. You'll get a lot of Infinity.

You can take a look into bigNumber.js or you can set an upper limit, like if (n > 1000) { fibDisplay.textContent = 'Too large'; return; } or you could store all values for your maximum n.

Calculating all numbers over and over again is some overhead you could avoid. You could cache already calculated values, and only add new values if n > values.length. Alternatively you can store all numbers beforehand, which is an increase in memory usage, but your runtime becomes incredibly fast, as all you have left is:

const values = [0, 1, 1, 2, 3, ...];
const output = values.splice(0, n);