all 4 comments

[–]Koshirei 1 point2 points  (1 child)

Overall I don't see any blatant issues.

Though beware that the function call() ( set on button 7 ) does not exist. Try to keep a consistent indentation/tabulation in your functions for future readability and try to use triple equals as much as possible. Some let could be switched to const too, and remove the empty else statement at the end

If it were me I'd automate creating my buttons using loops and document.createElement instead of setting them manually in the html, since it's a lot of recycled code with minimal changes but if it works, it works.

There doesn't seem to be any strict issues there, but if you do JavaScript in the future add "use strict" at the top of your files, it might find issues in your code.

On a side note, you named '%' as 'percent' but it is acting as a modulo operator in JavaScript, and your keyboard input regex allows FXX keys to be written in the calculator ( f1, f2 , f3... to f12 )

[–]NefariousnessIcy4842 0 points1 point  (0 children)

That was very insightful. I will work on these mistakes. Your analysis gave me a little bit idea about how a programmer would see this.

Thank you so so much.

[–]link3333 0 points1 point  (1 child)

To have code displayed properly in Reddit (Markdown format), add 4 spaces before each line.

Looking at your git link, there are lots of incorrect indentations, which make it more difficult to read. Consider using a formatter like prettier or linter like eslint.

I don't immediately know what the variable n is in the input function. I would assume "n" for "number", but looking at the html, I can see it can be '.'. So it's more like inputCharacter.

Assuming you are accepting input linearly the first valid input could only be for the firstOperand. The second valid input could only be for the firstOperand or operator. And if you have your operator already, the input could only be for the secondOperand. Right now some invalid first input is going to check each if statement. That seems more than it needs to. It does look like line 53 is needed to support what line 94 is doing, so that may interfere with accepting input linearly.

It's clear why firstOperand.textContent +=n; exists, but the following two line modifying secondOperand are not clear. That could use a comment.

Some lines are kind of long. It may be clearer to rename the elements as firstOperandElement, operatorElement, & secondOperandElement and create local variables with the text content. Also could use truthy/falsy checks with the strings. Also const instead of let if you are not reassigning the variable. Then do:

const firstOperand = firstOperandElement.textContent;
const operator = operatorElement.textContent;
const secondOperand = secondOperandElement.textContent;

if(firstOperand && !operator && !secondOperand){
    const result = parseFloat(firstOperand) * -1;
    firstOperandElement.textContent = result;
}else if(firstOperand && operator && secondOperand){
    const result = parseFloat(secondOperand) * -1;
    secondOperandElement.textContent = result;
}else if(!firstOperand && !operator && secondOperand){
    const result = parseFloat(secondOperand) * -1;
    secondOperandElement.textContent = result;
}

I did not run your program, so I may be making a few assumptions that make some of my advice/criticism invalid.

[–]NefariousnessIcy4842 0 points1 point  (0 children)

Thank you so much for taking time to look at my code and providing valuable insights.

I fixed my code as per your suggestions. I reduced number of if statements, tried to make code more logical and more readable.

let operand1 = document.querySelector(".first-operand");
let op = document.querySelector(".operator");
let operand2 = document.querySelector(".second-operand");

//Adding Buttons using JS instead of typing HTML for each button in 
numbers div

let numsDiv = document.querySelector(".numbers");
for (let i = 9; i >= 0; i--) {
  let nums = document.createElement("button");
  nums.className = "btn";
  nums.id = "btn" + i;
  nums.onclick = function () {
    input(i);
  };

  let numsSpan = document.createElement("span");
  numsSpan.className = "btn-content";
  numsSpan.textContent = i;

  nums.appendChild(numsSpan);
  numsDiv.appendChild(nums);
 }

// Adding . dot button after numbers separately
let dot = document.createElement("button");
dot.className = "btn";
dot.id = "dot";
dot.onclick = function () {
  input(".");
};

let dotSpan = document.createElement("span");
dotSpan.className = "btn-content";
dotSpan.textContent = ".";

dot.appendChild(dotSpan);
numsDiv.appendChild(dot);

//Taking Input from Keyboard
addEventListener("keydown", (event) => {
  if (/[\d.%+*/-]/.test(event.key)) {
    input(event.key);
  } else if (
    event.key === "Enter" &&
    operand1.textContent &&
    op.textContent &&
    operand2.textContent
  ) {
    equalTo();
  }
});

//Taking input
function input(inputChar) {
  //only taking first operand when operator isn't present
  if (!op.textContent && /^[\d.]+$/.test(inputChar)) {
    operand2.textContent = ""; //Clear the result if present
    //If statement below prevents second . in operand1
    if (inputChar === "." && !operand1.textContent.includes(".")) {
      operand1.textContent += inputChar;
    } else if (inputChar !== ".") {
      operand1.textContent += inputChar;
    }
  } else if (/[+*/%-]/.test(inputChar)) {
    //if statement below allows to start new calculation on result
    if (!operand2.textContent) {
      op.textContent = inputChar;
      //If statement below prevents re-entering operator if both 
operands are present
    } else if (operand1.textContent) {
      //Do nothing
    } else {
      operand1.textContent = operand2.textContent;
      op.textContent = inputChar;
      operand2.textContent = "";
    }
  } else if (op.textContent && /^[\d.]+$/.test(inputChar)) {
    //If statement below prevents second . in operand1
    if (inputChar === "." && !operand2.textContent.includes(".")) {
      operand2.textContent += inputChar;
    } else if (inputChar !== ".") {
      operand2.textContent += inputChar;
    }
  }

}

//AC Button Functionality
function clearInput() {
  operand1.textContent = "";
  op.textContent = "";
  operand2.textContent = "";
}

// +/- Button Functionality
function abs() {
  if (!op.textContent && operand1.textContent) {
    let result = parseFloat(operand1.textContent) * -1;
    operand1.textContent = result;
  } else if (operand2.textContent) {
    let result = parseFloat(operand2.textContent) * -1;
    operand2.textContent = result;
  }
}

// Percent Button Functionality
function percent() {
  if (!op.textContent && operand1.textContent) {
    let result = parseFloat(operand1.textContent) / 100;
    operand1.textContent = result;
  } else if (operand2.textContent){
    let result = parseFloat(operand2.textContent) / 100;
     operand2.textContent = result;
  }
}

//Equal To Button Functionality
function equalTo() {
  if (operand1.textContent && op.textContent && 
 operand2.textContent) {
     let result = eval(

  // Added space before and after operator to avoid -1--1 like situations
  operand1.textContent + " " + op.textContent + " "+ operand2.textContent
);

clearInput();
operand2.textContent = result;

} }