0

I am a 16 year old, very new to developing as a whole. Id say I been learning for about 5 days, I just want to know if there is any improvements I could implement or any problems with my code.

pragma solidity >=0.7.0 <0.9.0;

contract Vault is ReentrancyGuard {
    // State variables
    address public owner;

    constructor() {
        owner = msg.sender;
    }

    // Account details storage
    struct AccountDetails {
        string accountName;
        uint creationTime;
    }

    mapping (address => AccountDetails) accountDetails;
    mapping (address => uint256) totalDeposited;
    mapping (address => uint256) balances;

    // Deposit functiom
    function accountDeposit() public payable {
        if(accountDetails[msg.sender].creationTime == 0){
            require(msg.value > 0, "You must deposit more than 0.");
            balances[msg.sender] += msg.value;
            totalDeposited[msg.sender] += msg.value;

            AccountDetails memory details = AccountDetails("unnamed", block.timestamp);
            accountDetails[msg.sender] = details;
        } else {
            require(msg.value > 0, "You must deposit more than 0.");
            balances[msg.sender] += msg.value;
            totalDeposited[msg.sender] += msg.value;
        }
    }

    // Withdraw function
    function accountWithdraw(uint256 _amount) public payable nonReentrant {
        require(balanceOf(msg.sender) >= _amount, "No funds to withdraw.");

        balances[msg.sender] -= _amount;
        payable(msg.sender).transfer(_amount);
    }

    //Check the account balance of an address
    function balanceOf(address account) private view returns (uint256) {
        return balances[account];
    } 

    //change account name
    function changeAccountName(string memory _name) public {
         if(accountDetails[msg.sender].creationTime == 0){
            revert("You do not have an account, make a deposit to create an account.");
        }
        accountDetails[msg.sender].accountName = _name;
    }

    // View account name
    function accountName() public view returns (string memory) {
        if(accountDetails[msg.sender].creationTime == 0){
            revert("You do not have an account, make a deposit to create an account.");
        }
        return accountDetails[msg.sender].accountName;
    }

    // Check account balance
    function checkBalance() public view returns (uint256) {
        if(accountDetails[msg.sender].creationTime == 0){
            revert("You do not have an account, make a deposit to create an account.");
        }
            return balances[msg.sender];
        }

    // Check total deposit amount
    function checkTotalDeposit() public view returns (uint256) {
    if(accountDetails[msg.sender].creationTime == 0){
        revert("You do not have an account, make a deposit to create an account.");
    }
        return totalDeposited[msg.sender];
    }        

    modifier onlyOwner() {
        require(msg.sender == owner);
        _;
}
}

1 Answer 1

0

Looks awesome man(or woman)! And great to have you here.
I'd say add some events to the functions (best practice), and then in your deposit function, it looks like a lot of the logic is repeated in both the if/else portions, so you can take a lot out of the conditional to make it smaller. Looks like you can also remove the owner since you don't use it for anything.

1
  • Thanks for pointing out the if else problem appreciate it !!!! Apr 12 at 19:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.