Skip to content

Elad abutbul calcualtor#16

Open
Elad-Abutbul wants to merge 2 commits into
ColmanDevClubORG:mainfrom
Elad-Abutbul:elad-abutbul-calcualtor
Open

Elad abutbul calcualtor#16
Elad-Abutbul wants to merge 2 commits into
ColmanDevClubORG:mainfrom
Elad-Abutbul:elad-abutbul-calcualtor

Conversation

@Elad-Abutbul

Copy link
Copy Markdown

No description provided.

@Tamir198 Tamir198 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left you some comments

Comment thread script.js
let firstNum = 0;
let secondNumb = 0;
let operator = null;
let bool = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give this more meaningful name? what is the roll of this bool?

Comment thread script.js

buttons.addEventListener("click", (e) => {
const el = e.target;
if (el.tagName !== "BUTTON") return;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The string should be saved inside constant variables

Comment thread script.js
const el = e.target;
if (el.tagName !== "BUTTON") return;
const text = el.innerText;
if (text === "C") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this and all occurences

Comment thread script.js
calculate();
return;
}
if (el.className === "") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add else if and not just if to prevent check every if statement

Comment thread script.js

function appendDigit(el) {
const val = Number(el.innerText);
if (Number.isNaN(val)) return;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good practice

Comment thread script.js
const val = Number(el.innerText);
if (Number.isNaN(val)) return;
if (bool) {
firstNum = firstNum * 10 + val;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Save 10 into a variable
Also you can add return statement under this line to avoid writing the if

Comment thread script.js
res = firstNum - secondNumb;
break;
default:
return;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You dont really need empty default, if no case is met the function will simply terminate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants