Data Location Assignment [OLD]

I changed the updateBalance function to the following:

    function updateBalance(uint id, uint balance) public {
         users[id].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];
     users[id].balance = balance;
}

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

}

The change I made was to remove the commented line above. Of course commenting the line above removes it!

At first I changed the word ‘memory’ to ‘storage’ because we want to make a permanent change to the mapping, rather than a change that only persists for the duration of the code execution. This solution does work, but you get a warning message from the compiler rather than a green tick. Removing the line all together I found was the best solution. That way, the code makes full use of the mapping function defined in the first few lines.

1 Like

Just an FYI for some who may be having compiling difficulty. When I made the change to the addUser function from memory to storage, It kept showing an error on that line. After checking and rechecking,and trying a couple of things, I tried running the pragma solidity 0.5.1 contract with the 0.7.0 (latest) compiling format and the problem was solved! :grinning:

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

pragma solidity 0.5.1;
contract MemoryAndStorage {

mapping(uint => User) users;

struct User{
    uint id;
    uint balance;
    bool exist;
}

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

function updateBalance(uint id, uint balance) public {
    // check if the userbalance actually exists
    require(
        users[id].exist,
        "Account does not exist."
    );

    //users[id].balance = balance; <-- first solution to updating the balance.

    // Second solution of updating the balance.
    User storage user = users[id];
    user.balance = balance;
}

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

}

I came up with this solution, first i just added the balance directly to the mapping, then I tried to change memory to storage, now it updated the balance. But it also updates balances of non existing accounts… So i added a boolean to the struct that gets set to true when a user is added. I use that to check if an account actually exists when updating the balance.

2 Likes

Hi @Uyan,

Your solution is correct :ok_hand:

You don’t get a warning :warning: if you keep the original second line of code, and only change memory to storage in the first line i.e.

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

By changing the second line to the solution that works as a single line on it’s own…

users[id].balance = balance;

… you are referencing the mapping users , and therefore not using user (the new User instance, created in the first line). If you look at the actual warning message, you will see that that’s the reason given.

I hope that’s clear :slightly_smiling_face:

2 Likes

Hey @Rolf,

Well done for spotting the weakness, and for coming up with a suitable solution/improvement :+1:

Nicely, done! :smiley:

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);
    }

    function updateBalance(uint id, uint _balance) public {
         users[id].balance = _balance; //the variable before was just the same with
                       // the balance var above so I made a var that only exists in this function
    }

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

}

Not sure if I’m understanding the topics correctly. Please tell me if I’m missing something.

1 Like

The simple solution is to change that user to be saved in storage.

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

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;                                 // works with this change to updateBalance function
}

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

}

1 Like

It could be both:

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;}

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

or

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

One solution is to directly change the value of users[id].balance instead of having to create a local variable “user”.

Or you change “memory” to “storage” data location…

1 Like

My solution:

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

Hi @newavel,

Your solution is correct, so it seems that you have understood the concepts introduced in this section correctly :slightly_smiling_face:

If you have any specific points that you are uncertain about, then try to express them in a post here, together with the relevant code and we’ll help you out.

Both balance parameters are only used locally within two separate functions. Their scope is local to the function they are passed to, and so they cannot be accessed outside of their specific function.

The variable balance which is declared within the User struct can only be accessed as
(i) a property of User , either directly, or indirectly via the users mapping; or
(ii) a property of an new instance of User e.g.
User memory user;
user.balance;

As a result of this, there is no problem and no conflict generated by using the same name balance for each of these.

However, there is a certain logic to your use of a name variation _balance for the new balance to update. I like how your code distinguishes between this new balance (only with local scope), and the account balance (available globally via the struct), to which the new balance is then assigned, replacing the previous one (even though they are in effect the same thing). I think it makes the statement users[id].balance = balance;  clearer as to what each balance is referring to, and therefore easier to interpret, although as I mentioned above, it’s not needed for the contract to execute correctly.

1 Like

Hi @Kristhofer_Drost1,

Great alternative solutions :ok_hand:

Just a word of caution about the layout you’re using with the positioning of the curly brackets, more than one statement per line, and no indentation. It’s not wrong, and your contract will still execute, but as your layout is unconventional, it’s actually harder to read and follow. This isn’t such a problem with such a short contract, but could become an issue with much larger contracts.

Personally, I think it could also lead to missing more errors with certain things like brackets. Although, I admit, that does depend on what you’re used to. Your second alternative doesn’t execute, as you’ve missed a curly bracket at the end of the updateBalance function.

2 Likes

That does sound right, @ajphminer, but could we see your actual code just to be sure :wink:

1 Like

Hi Matas,

Great to see you’re progressing through Solidity, now! I hope all is going well :slightly_smiling_face:

Good solution to this assignment by the way :ok_hand:

1 Like

The other answer would have got me in trouble with my last boss.

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

Thank you for the advice. I’ll keep an eye on it :slight_smile:

1 Like
pragma solidity 0.5.1;

contract MemoryAndStorage {

    struct User{
        uint balance;
        
        bool exist;
    }
    
    mapping(uint => User) users;
    
    modifier _checkId(uint id, bool state) {
        string memory error_msg;
        
        if (state) {
            error_msg = "The ID does not exists";
        } else {
            error_msg = "The ID already exists";
        }
        
        require(users[id].exist == state, error_msg);
        _;
    }
    
    function addUser(uint id, uint balance) public _checkId(id, false) {
        users[id] = User(balance, true);
        
        assert(users[id].balance == balance);
        assert(users[id].exist == true);
    }

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

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

}
1 Like

Hi @Dan_O_Kane,

Would be interesting to know why? :smile:
The solution you’ve given is the most concise and uses less gas, so I’m guessing it could be either of those 2 reasons…

1 Like