Data Location Assignment [OLD]

Just switched the Memory data location with a storage, now thinking about it maybe storing it in the mapping would be more efficient.

pragma solidity 0.5.1;
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 memory user = users[id];
     user.balance = balance;
}

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

}

1 Like

Hi @gmarcotte,

Nice solution :ok_hand: … and it’s also great how you’ve extended the assignment to address a weakness in the original basic contract, so that now new users without an existing account have to add an account before they can update their balance :+1:

Because the require statement you’ve added to addUser prevents new users creating accounts with zero IDs, you don’t need to also include this same constraint in updateBalance. All existing users will have IDs greater than zero, and so as   require(users[id].id > 0);   checks that the user already exists, all user IDs passing this requirement will also be greater than zero.

You can also include the same require statement  require(users[id].id > 0);   in the getter, to prevent zero balances being returned to users without an existing account.

I think it would also be a good idea to add some appropriate error messages to your different require statements. This will make the different reasons for a transaction failing clearer to the caller.

You may also want to think about how to prevent existing users from having their account balance overwritten by a new user creating an account with the same id, which is another weaknesses in the original contract.

I think here you mean  //user.balance = balance;:wink:

Keep up the great work! You’re making good progress :muscle:

1 Like

Hi @Pigott,

Welcome to the Academy forum! Good to see you here! :smiley:

Correct… you’ve just forgotten to make that change in the code you’ve posted :wink:

In actual fact, by changing memory to storage you already are storing the updated balance in the mapping:
User storage user = users[id];  effectively makes user a direct reference (a “pointer”) to the User instance (with that ID) stored permanently in the mapping at users[id] . This means that the second line  user.balance = balance;  also now assigns balance to the mapping, and so does exactly the same as the one line solution  users[id].balance = balance;
I think this is probably why the first solution hardly uses any more gas than the second; and when optimization is enabled, there is, in fact, no difference in gas cost. However, I do think that the second solution has the advantage of being more concise.

1 Like

solution ‘memory’ in solidity will not reflect in the original copy, therefore, changing storage location from memory to ‘storage’ creates a pointer to storage, and any change in the user object will be reflected in the storage itself.

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

Just change the word memory for storage in the update balance function

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

1 Like

We want to save the balance & id mappings permanently, not only for function use, so changing memory to storage seems like a logical solution.

1 Like

image

1 Like

Yes, essentially that’s correct, @Lucus.

Just a comment about this, though…
The id parameter is not being saved here. As the mapping key, it is used to “find” the User instance in the mapping we want to update the balance for.
User storage user = users[id]; effectively makes user a direct reference (a “pointer”) to the User instance (with that ID) already stored permanently in the mapping at users[id] . This means that the second line user.balance = balance; now saves the new balance permanently in the mapping (updating it), and doesn’t only assign it temporarily to the local variable user.

2 Likes

So, changing memory to storage is allowing the balance to be updated when users interact and this causes the mapping to update it’s pointer?

1 Like

pragma solidity 0.5.1;

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; //(struct.property) users[id].balance is already in storage scope
}

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

}

1 Like

I did my research and I found two solutions, for me a clearer way to do this exercise was to update the balance directly in the mapping, then I found the other solution to change the data location from memory to storage.

// this function takes uint id & uint balance as arguments and updates balance
    function updateBalance(uint id, uint balance) public {
        // 1st solution: we can update balance by changing it directly in the mapping 
        users[id].balance = balance;
      // 2nd solution: When we have data location "memory"  the balance is not permanently stored and won't be updated.
      // we need to change data location to "storage" to be able to update any changes in User struct 
       User storage user = users[id];
       user.balance = balance;
      
    }
1 Like

pragma solidity 0.5.1;
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

Hi,

I was just wondering when we see any withdraw function which is onlyOwner, does that may mean is an exit scam scmart-contract? if not, besides for secutiry reasons onlyOwner and not a “normal” user is intended to withdraw the funds, is it normal this function is written in a DeFi token?

Just wondering, I know you have Secutiry Contract. course but haven’t get there yet.

Thank you in advance for your answers.

1 Like

Solution 1:
function updateBalance(uint id, uint balance) public {
// directly set balance in the mapping
users[id].balance = balance;
}
Solution 2:
function updateBalance(uint id, uint balance) public {
// change the user variable to storage type, will then update users mapping
User storage user = users[id];
user.balance = balance;
}

1 Like

amended the updateBalance function to:

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

Yes

This causes a specific user’s balance to be updated in the mapping. The “pointer” isn’t something that the mapping “has”. It is a reference to a particular place in the mapping, from somewhere else in the program.

users[id]  is the “standard” way to “point to” a specific User instance (a user’s record) in the users mapping, because the ID is the key to that specific User instance. [id] references the function’s id parameter which is input by the function caller, and represents the user whose balance we want to update.
So, probably the most obvious (and easiest to understand) way to update a user’s balance is as follows:

users[id].balance = balance;

balance to the right of the assignment operator (=) references the function’s balance parameter which is input by the function caller, and represents the new balance.
This new balance is assigned to  users[id].balance  which is effectively a pointer to where the new balance should be assigned. As explained above users[id] points to the “record” of a specific user in the users  mapping, and by adding the balance property, it now points to the specific value held in that property which represents the user’s existing balance. As a result, the user’s balance stored in the mapping is updated.

With   User memory user = users[id]   the pointer itself is assigned to a local user variable, which means that a copy is made, and stored locally in memory, of the specific user’s record which the pointer references in the mapping. After being assigned, the pointer users[id] is no longer used within the function. So instead of the mapping being updated with the new balance,  user.balance = balance  updates the local copy with the new balance. This update is lost when the function is exited, because the change has not been stored permanently in the mapping, but instead only temporarily in memory.

With   User storage user = users[id]   the pointer is assigned to the variable user, and because the data of both user and the mapping users is located in storage, this effectively creates a copy of the pointer, and means that within the function both users[id] and user will act as identical pointers to the same user’s record in the mapping. This is why, in their own specific context, the following lines of code both update a specific user’s balance in the mapping:

users[id].balance = balance;
user.balance = balance;

Sorry, about the long answer, but it’s very hard to answer your question without explaining the details of what’s actually happening. Anyway, I hope this helps to make it a bit clearer. Understanding these types of concepts is often a gradual process, so don’t be surprised if you still don’t immediately understand this fully. With more study and practice, you’ll then probably find that later on things will suddenly fall into place and click :slight_smile:

1 Like

Nice solution @alexsei :ok_hand:

Just to clarify…

…the balance property is not updated in the actual struct, but in a specific instance of that struct within the mapping. I’m sure this is what you have understood, but I just wanted to clarify it.

1 Like

A nice analysis of two alternative solutions @MrAndres :ok_hand:

Just to clarify…

…the balance is not updated in the actual struct, but in a specific instance of that struct within the mapping. I’m sure this is what you have understood, but I just wanted to clarify it.

2 Likes

pragma solidity 0.5.1;
contract MemoryAndStorage {

mapping(uint => User) users;

struct User{
    uint id;
    uint balance;
}

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

//The "user" object is in memory so it will be lost after execution. In order to updated the balance in storage, we should updated the user in "users" array.
function updateBalance(uint id, uint balance) public {
     User memory user = users[id];
     user.balance = balance;
     users[id] = user;
}

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

}

1 Like

pragma solidity 0.5.1;
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 memory user = users[id];
     user.balance = balance;
     users[id] = user;
    
}

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

second solution
pragma solidity 0.5.1;
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;
}

}
I prefer the first one to have memory storage for the user instead storing it permanently.

1 Like