refactor decimal to binary converter to check for positive numbers only

[code-liana]

Describe the Issue

STEPS TO REPRODUCE:

1. Add a negative number to your app

RESULT:
Negative numbers are displayed as isNaN = false but the app displays NaN on the result field.
The user can see on the console if the application running through the website of freeCodeCamp
Potential infinite loop detected on line 32. Tests may fail if this is not changed.

If I copy html, css and js file to vc and run it stand-alone on the browser I see this on the console

Uncaught RangeError: Maximum call stack size exceeded
    at decimalToBinary (script.js:33:3)
    at decimalToBinary (script.js:36:12)
    at decimalToBinary (script.js:36:12)
    at decimalToBinary (script.js:36:12)
    at decimalToBinary (script.js:36:12)
    at decimalToBinary (script.js:36:12)
    at decimalToBinary (script.js:36:12)
    at decimalToBinary (script.js:36:12)
    at decimalToBinary (script.js:36:12)
    at decimalToBinary (script.js:36:12)
decimalToBinary @ script.js:33
decimalToBinary @ script.js:36
decimalToBinary @ script.js:36
decimalToBinary @ script.js:36
decimalToBinary @ script.js:36
...

Affected Page

https://www.freecodecamp.org/learn/javascript-algorithms-and-data-structures-v8/learn-recursion-by-building-a-decimal-to-binary-converter/step-109

Your code

const numberInput = document.getElementById("number-input");
const convertBtn = document.getElementById("convert-btn");
const result = document.getElementById("result");
const animationContainer = document.getElementById("animation-container");
const animationData = [
  {
    inputVal: 5,
    marginTop: 300,
    addElDelay: 1000,
    msg: 'decimalToBinary(5) returns "10" + 1 (5 % 2). Then it pops off the stack.',
    showMsgDelay: 15000,
    removeElDelay: 20000,
  },
  {
    inputVal: 2,
    marginTop: -200,
    addElDelay: 1500,
    msg: 'decimalToBinary(2) returns "1" + 0 (2 % 2) and gives that value to the stack below. Then it pops off the stack.',
    showMsgDelay: 10000,
    removeElDelay: 15000,
  },
  {
    inputVal: 1,
    marginTop: -200,
    addElDelay: 2000,
    msg: 'decimalToBinary(1) returns "1" (base case) and gives that value to the stack below. Then it pops off the stack.',
    showMsgDelay: 5000,
    removeElDelay: 10000,
  }
];

const decimalToBinary = (input) => {
  if (input === 0 || input === 1) {
    console.log("input ", input)
    console.log("input === 0 ", input === 0)
    console.log("input === 1 ", input === 1)

    return String(input);
  } else {
    const result = decimalToBinary(Math.floor(input / 2)) + (input % 2);
    console.log("result = >>>>>> ", result)
    return result
  }
};

const showAnimation = () => {
  result.innerText = "Call Stack Animation";

  animationData.forEach((obj) => {
    setTimeout(() => {
      animationContainer.innerHTML += `
        <p id="${obj.inputVal}" style="margin-top: ${obj.marginTop}px;" class="animation-frame">
          decimalToBinary(${obj.inputVal})
        </p>
      `;
    }, obj.addElDelay);

    setTimeout(() => {
      document.getElementById(obj.inputVal).textContent = obj.msg;
    }, obj.showMsgDelay);

    setTimeout(() => {
      document.getElementById(obj.inputVal).remove();
    }, obj.removeElDelay);
  });

  setTimeout(() => {

  }, 20000);
};

const checkUserInput = () => {
  const inputInt = parseInt(numberInput.value);
    console.log("inputInt=>>>>>", inputInt, " isNaN =>>>> ",isNaN(inputInt))
  if (!numberInput.value || isNaN(inputInt)) {
    alert("Please provide a decimal number");
    return;
  }

  if (inputInt === 5) {
    showAnimation();
    return;
  }

  result.textContent = decimalToBinary(inputInt);

  numberInput.value = "";
};

convertBtn.addEventListener("click", checkUserInput);

numberInput.addEventListener("keydown", (e) => {
  if (e.key === "Enter") {
    checkUserInput();
  }
});

Expected behavior

As far as I read on Google negative numbers can be converted to binary but it's the whole process,
Here is a clear example of how to do it at the bottom of the article: https://sciencing.com/calculate-binary-numbers-8267989.html

Screenshots

RESULT ON FREECODECAMP

RESULT WHEN COPIED TO LOCAL

System

Device: Laptop
OS: Windows 11 Home
Browser: Chrome
Version: 124.0.6367.61

Additional context

No response

[Asadali00000]

hello ,
i want to ask ,
what output do you prefer for negative numbers for example :- take the number
"-7"
1st->-0111(this will be easy and changes in code will be less)
2nd->1001(2's complement , This method requires additional code. we can also try to make this recursive (non recursive is easy to understand ,clean and efficient but its recursion tutorial) because we have to show recursive stack animation ).

"Please reply to this. What output do you prefer for negative numbers?"

[code-liana]

@Asadali00000
The first option is easy but I think that it's not right to teach people to do anything the wrong way. Campers come here to learn and they trust the system and what is given to them.

What output is preferred is the question to those brilliant minds of the people who created and worked on this project in the first place.

I'm still on the stage of making my brain comprehend how binaries work :) and it all thanks to this project to begin with.

[jdwilkin4]

If I remember correctly, I believe the original design was to only account for positive numbers.
@scissorsneedfoodtoo will probably know better about that then me.

But IMO, I think we should just add a note in the beginning for step 1 that this project will build out a binary converter for positive numbers.

Because the real goal is to reach recursion and not so much of teaching an in depth lesson on binary if that makes sense.

I will open this up for discussion so the rest of the team can chime in

[scissorsneedfoodtoo]

Thanks for bringing this to our attention, @code-liana, and for taking the time to open up this issue.

As cool as it would be to update the algorithm to work with negative numbers, I agree with @jdwilkin4 that this project should focus more on recursion rather than going too much into decimal to binary conversion.

Like Jessica mentioned, we could adjust the text in step 1 to emphasize that this project will only convert positive numbers into binary. And while this would involve more changes, we could adjust the first if statement in the checkUserInput function to check if the number entered is less than 0, and tweak the alert message slightly.

Towards the beginning of the project the if statement could be something like the following:

if (!numberInput.value || isNaN(parseInt(numberInput.value)) || parseInt(numberInput.value) < 0) {
  alert("Please provide a decimal number greater than or equal to 0");
  return;
}

Then later after refactoring to use the inputInt variable, it could be something like:

if (!numberInput.value || isNaN(inputInt) || inputInt < 0) {
  alert("Please provide a decimal number greater than or equal to 0");
  return;
}

That seems like an easier solution than adding in another if statement to that function, and should prevent the loop protection from triggering.

[jdwilkin4]

I like that approach @scissorsneedfoodtoo

Let's go ahead and open this up for contribution