Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Develop #2319

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Develop #2319

wants to merge 3 commits into from

Conversation

Barosz30
Copy link

@Barosz30 Barosz30 commented Nov 9, 2023

No description provided.

Copy link

@MarcinLigmann MarcinLigmann left a comment

Choose a reason for hiding this comment

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

the comments are just my suggestions, the code looks good and works.
I approve :)

if (counter > 2 && counter % 2 === 1) {
counter++;

return `Bzzz... Error!`;

Choose a reason for hiding this comment

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

would wrap it in a variable, e.g. error, then return error

Choose a reason for hiding this comment

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

Actually, since we are not doing operations on this variable, it does not change or appear in different places, it's not a part of a list of options (like in stateful clones) we don't need to wrap it in a variable.

} else {
counter++;

return a + b;

Choose a reason for hiding this comment

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

here I would also wrap in a variable sum and return the sum

Choose a reason for hiding this comment

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

same as above

Comment on lines 13 to 27
function getSum(a) {
function bullshitNesting(b) {
if (counter > 2 && counter % 2 === 1) {
counter++;

return `Bzzz... Error!`;
} else {
counter++;

return a + b;
}
}

return bullshitNesting;
}

Choose a reason for hiding this comment

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

You can just use arrow functions () => {} instead off naming your functions bullshitNesting. Please remember to avoid such Easter eggs.

if (counter > 2 && counter % 2 === 1) {
counter++;

return `Bzzz... Error!`;

Choose a reason for hiding this comment

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

Actually, since we are not doing operations on this variable, it does not change or appear in different places, it's not a part of a list of options (like in stateful clones) we don't need to wrap it in a variable.

} else {
counter++;

return a + b;

Choose a reason for hiding this comment

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

same as above

function getSum(a) {
function bullshitNesting(b) {
if (counter > 2 && counter % 2 === 1) {
counter++;

Choose a reason for hiding this comment

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

move counter++ outside of if statements

counter++;

return `Bzzz... Error!`;
} else {

Choose a reason for hiding this comment

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

if you are using if with return there is no need for else keyword

if {
  //code
  return 
}

return // some other code 

Copy link

@mvjl000 mvjl000 left a comment

Choose a reason for hiding this comment

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

Looks good!

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.

4 participants