Data Location Assignment

Ethereum Smart Contract Programming 101
#631

Nice solution @JJ_FX :ok_hand:

The additional admin state variable, constructor and modifier you’ve added, which only allows the deployer of the contract (admin) to add new users and update their balances, also makes sense :+1: Your modifier works wherever its declaration is positioned within the contract body, but I think the overall structure and functionality of your code will be clearer if the declaration appears before its implementation in the functions. When reading the code of a contract, it makes more intuitive sense to have seen the modifier’s actual code before seeing references to it in the function headers.

Even though the initial idea of this assignment is to update the existing balance by replacing it with the new one, I actually think that it makes more sense to do what you have done, and add an amount to the existing balance using the addition assignment operator += (instead of the assignment operator = ). However, if you add to , instead of replacing , the existing balance, I think your code would be clearer and more readable if you also change the name of the balance parameter to amount , because you are adding an amount to the balance, rather than replacing it with a new balance .

Let me know if you have any questions :slight_smile:

1 Like
#633

Thanks @jon_m :smiley:

Yeah you’re right, renaming balance to amount makes much more sense!

EDIT: About the contract structure: So I come from Unity C# scripting. And I got used to keep additional functions declarations on the bottom of the script, while keeping the core logic at the top (maybe except costructor, as it’s called first). Ideally would be to just decouple (not sure I’m using that word correctly) the modifier, and maybe constuctor, from the main logic. But just now I learned about inheritance in solidity :sweat_smile: So that’s how I would probably do that next time!

1 Like
#634

Hi @JJ_FX,

Personally, I don’t have any Unity C# experience, so I can’t comment on how the structure of the code in smart contracts in Solidity compares (@thecil can you comment on this?) But, in smart contracts, I would argue that modifiers (along with state variables, mappings, event declarations and constructors) are also part of the core logic which the code in the contract’s functions depends upon, and so should be positioned towards the top of the contract.

I think another good way to think about this is in terms of code reusability and modularity. Any “chunks” of code, such as modifiers, which by their nature are designed to be reused and to reduce code duplication in other parts of the contract, should generally be positioned towards the top of the contract. And as I think you have already realised …

… “chunks” of code which, as well as reducing code duplication within a single smart contract, can also be reused across multiple contracts, are better moved to a separate base contract and made available in derived contracts via inheritance. Is that what you were meaning?

#635

this is my solution:

pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User{
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance) public {
        users[id] = User(id, balance);
    }

    function updateBalance(uint id, uint balance) public {
         users[id].balance = balance;
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}
1 Like
#636

The issue was within the function which updates the balance. It created a local version of the user, then updated that users balance. However since it’s a local version it just dissapeared when the function ended. Instead either use the solution below, or save the local user struct to the users list.

    // Update the balance.
    function updateBalance(uint id, uint balance) public {
         users[id].balance = balance;
    }
1 Like
#637

Nice solution @anthony_salameh :ok_hand:

… and welcome to the forum! I hope you’re enjoying the course :slight_smile:

#638

Nice solution @Lennart_Lucas :ok_hand:

… and a good explanation of the problem and alternative solution :+1:
Just one comment …

This is correct… but more accurately …

“… or save the local user struct instance to the users mapping.”

1 Like
#639

storage = Persistent data storage
memory = Temporary data storage

pragma solidity 0.7.5;

contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User {
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance) public {
        users[id] = User(id, balance);
    }

    function updateBalance(uint id, uint balance) public {
         User storage user = users[id];
         user.balance = balance;
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}
1 Like
#640

Greetings!! I cannot get the code to work properly in Remix. addUser works, but getBalance returns the original balance from addUser and not the updateBalance one. I’ve run this many times. I’ve run the other version of the updateBalance function that Filip provided.

The responses in the console are confusing, but I can see that the function didn’t complete. (see below contract code)

pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User{
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance) public {
        users[id] = User(id, balance);
    }

    function updateBalance(uint id, uint balance) public {
         User storage user = users[id];
         user.balance = balance;
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}

Response in Console:

call to MemoryAndStorage.getBalance
 
CALL
[call]from: 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4to: MemoryAndStorage.getBalance(uint256)data: 0x1e0...0000b
status	undefined 
transaction hash	0xf5ad670b4b47d334820d33f3699f3a4f1ef0e4e362d02254a84543f0a4fb1968
from	0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
to	MemoryAndStorage.getBalance(uint256) 0xd9145CCE52D386f254917e481eB44e9943F39138
execution cost	23666 gas (Cost only applies when called by a contract)
hash	0xf5ad670b4b47d334820d33f3699f3a4f1ef0e4e362d02254a84543f0a4fb1968
input	0x1e0...0000b
decoded input	{
	"uint256 id": {
		"_hex": "0x0b",
		"_isBigNumber": true
	}
}
decoded output	{
	"0": "uint256: 33"
}
logs	[]
1 Like
#641

Hi @13zebras,

Your solution is coded correctly, and when I deploy it in Remix it works as it should. I can see from the call receipt you’ve posted that you were using an ID of 11, and updating the user’s balance to 33. So, for example if we do the following…

  1. Call addUser with  11, 22

  2. Then, call getBalance with  11
      ==>   the user’s initial balance of 22 is returned in the Remix panel below the blue getBalance button

  3. Call updateBalance with  11, 33

  4. Then, call getBalance with  11
      ==>   the user’s updated balance of 33 is returned in the Remix panel below the blue getBalance button. This will also generate the same call receipt in the Remix console that you got (except for a different contract address and different hash numbers). The receipt you’ve posted is what we should expect when we call getBalance after calling updateBalance with an id argument of 11 and a balance argument of 33.

The new balance that is input replaces the original balance; it isn’t added to it. So the updated balance is 33, not 55 (22 + 33). If you want to add the input amount to the existing balance you would need to change the assignment operator from = to += in the following line of code …

user.balance += balance;

Even though the initial idea of this assignment is to update the existing balance by replacing it with the new one, I actually think that it makes more sense to add an amount to the existing balance, and so maybe this is what has led to you thinking that your code isn’t working…

However, if we add to, instead of replacing, the existing balance, I think the code would be clearer and more readable if we also change the name of the balance parameter to amount, because we are adding an amount to the balance, rather than replacing it with a new balance  e.g.

function updateBalance(uint id, uint amount) public {
    User storage user = users[id];
    user.balance += amount;
}

Let me know if that clarifies things, or whether you’re still having problems or have any further questions :slight_smile:

#642

Hello Jon… Thanks for the reply. I should have been clear that getBalance only returns the initial balance from addUser, and never returns the updateBalance amount no matter how many times I run the updateBalance function. Example:

addUser: 23, 44
getBalance: 23 // displays 44 as expected
updateBalance: 23, 67
getBalance: 23 // displays 44 again
updateBalance: 23, 35
getBalance: 23 // displays 44 again

Sooooo… I did the above in Brave Browser running on Linux. I then opened Remix in Firefox on Linux. Copied oode to new .sol file. And voila!! It worked like a charm!!

SOMETHING IN BRAVE, maybe only on Linux, is blocking proper execution of functions in Remix. I’ll test on my Mac, too, to see if the issue is Brave.

Damn, that was frustrating!!

1 Like
#643

Long story short: I use an extension in Brave that was causing some sort of conflict. I’ve completely sorted it out now. Thanks for your help! :man_facepalming:

1 Like
#644

Ufff, yeh, that does sound frustrating,Tom!

That’s the first I’ve heard of that particular problem, although I do know Brave has caused some issues for students in other parts of the Ethereum Smart Contract Programming courses…

@thecil, the problems with Brave are often to do with connecting to Metamask, aren’t they? Is it also best not to use Remix in Brave? Am I right in thinking that these sorts of problems with Brave can often be solved by adjusting certain settings, or clearing the cache or similar?

Anyway, I’m glad you got it working in the end, Tom! :sweat_smile:

#645 
mapping(uint256 => User) users;
    
struct User {
  uint256 id;
  uint256 balance;
}
    
function addUser(uint256 _id, uint256 _balance) external {
  users[_id] = User(_id, _balance);
}
    
function updateBalance(uint256 _id, uint256 _balance) external {
  User storage user = users[_id];
  user.balance = _balance;
}
    
function getBalance(uint256 _id) view external returns (uint256 userBalance) {
  return users[_id].balance;
}
1 Like
#646

Hey Markus! Good to see you back here! :slightly_smiling_face:

You’ve made some nice improvements to the code for this assignment :ok_hand:

1 Like
#647

Hey Jon! Thanks, happy to be back too :grin:

1 Like
#648 
pragma solidity 0.7.5;
contract MemoryAndStorage {

    mapping(uint => User) users;

    struct User{
        uint id;
        uint balance;
    }

    function addUser(uint id, uint balance) public {
        users[id] = User(id, balance);
    }

    function updateBalance(uint id, uint balance) public {
         User storage user = users[id];
         user.balance = balance;
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }

}

So basically I only changed memory to storage in the updateBalance function.

1 Like
#649

pragma solidity 0.7.5;
contract MemoryAndStorage {

mapping(uint => User) users;

struct User{
    uint id;
    uint balance;
}

function addUser(uint id, uint balance) public {
    users[id] = User(id, balance);
}

function updateBalance(uint id, uint balance) public {
     users[id].balance = balance;
}

function getBalance(uint id) view public returns (uint) {
    return users[id].balance;
}

}

1 Like
#650

pragma solidity 0.7.5;
contract MemoryAndStorage {

mapping(uint => User) users;

struct User{
    uint id;
    uint balance;
}

function addUser(uint id, uint balance) public {
    users[id] = User(id, balance);
}

function updateBalance(uint id, uint balance) public {
     User storage user = users[id]; // Just replace memory to storage cuz u dont want to create a new instance, but to point a the one on the mapping //
     user.balance = balance;
}

function getBalance(uint id) view public returns (uint) {
    return users[id].balance;
}

}

1 Like
#651

Nice solution @ItsJackAnton :ok_hand:

… and a good explanation.