Skip to content

feet:add increase counter #3

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

NUMBER72857
Copy link

PR Checklist

  • Tests
  • Documentation

function transferOwnership(address _newOwner) public onlyOwner {
require(_newOwner != address(0), "Invalid address");
owner = _newOwner;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could emit a TransferOwnership event ☘️

require(bytes(_newName).length > 0, "Name cannot be empty");
students[msg.sender].name = _newName;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 LGTM

/// @param _value The amount to increase the counter by.
function increaseByValue(uint _value) public {
require(count <= type(uint).max - _value, "cannot increase beyond max uint");
count += _value;
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 Nice Logic to check for Overflow here.

uint public count;
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.24;

Copy link
Collaborator

Choose a reason for hiding this comment

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

👎
Not sure what you got going on here, but it isn't what I think you want to do that you're doing here.

You already declared the pragma and contract Above, Why are you repeating it again, in your contract block?

}
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

👎
The pragma, contract comment above is followed up here.

Please fix the requested changes, as this would not compile.

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