Data Location Assignment

Nice solution @firequo :ok_hand:

… and it’s good to see you back here in the forum! I hope you’re enjoying the course :slight_smile:

Hi @Tomaage ,

Your solution has made the updateBalance function exactly the same as the addUser function…

While this does still work…

I assume you’ve already seen the 2 alternative solutions suggested in the follow-up video. Can you see how they are more suitable?

You may also find this related discussion interesting, which starts in the post I’ve linked to and continues over several more.

Just let me know if you have any questions :slight_smile:

1 Like

Thanks Jon . I posted before I saw the solution because if it was something flawed I did not see, its good to have this forum point it out to me so I learn from it.

When I did enginering school, we had a hydrolics enigneer who would always make us install our test equipment the worng way, or les optimal wasy, so we we could see the mistake when we turned it on and everything was shaking and making strange noises, and then understand better why the correct way was more optimal.

1 Like

Yes… posting your own solution, and then seeing if the feedback you get matches what you realised yourself when you checked the solution code, and having a good think through any additional points that are raised … is a great way to learn :+1:

@jon_m thank you for help describe concept.My logic not well much.
Yes,I already see the 2 alternative solutions suggested in the follow-up video.It use just property inside users.Not replace value as I do.

        //solution 1

        users[id].balance = balance;

         //solution 2

         User storage user = users[id];

         user.balance = _balance;
1 Like

Hi @thanasornsawan,

Both your solution and the ones shown in the follow-up video replace values. The difference is that…

  • Your solution replaced the whole User struct instance (the value mapped to an address key in the users mapping). This isn’t wrong, but as you’ve understood, it’s unnecessary.

  • The two alternative solutions shown in the follow-up video replace just the uint value stored in the struct instance’s balance property.

I’m sure this is what you’ve understood, but I just wanted to clarify the correct terminology.

I hope that’s helpful :slight_smile:

no idea but I gave it my best shot and it seems to be working to me… lol Moving on to see others and the solution in next video :slight_smile:

1 Like

Nice solution @casadcode :ok_hand:

Just make sure that you use the more recent Solidity version 0.7.5 (as used in the course videos). It doesn’t make any difference for this assignment, but it will for the rest of the course.

Also, for your next assignments, please don’t post screen shots, because we can’t copy and paste your code in order to execute and test it, if we need to. Instead, follow the instructions in this FAQ: How to post code in the forum

Let me know if you have any questions :slight_smile:

1 Like
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

My try on this assignment:

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

Hello there, here 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 {
        // "memory" will trigger a copy of the struct
        // I can save the previous balance just to assert
        // that the copy is indeed a valid copy
        User memory user = users[id];
        uint pre_balance = user.balance;
        assert(pre_balance == users[id].balance);
        require(pre_balance != balance, "Updating with same balance");

        // solution 1:
        // I can avoid problems using the full notation
        users[id].balance = balance;
        assert(
            users[id].balance == balance &&
            users[id].balance != pre_balance // not really needed but more clear
        );

        // restoring pre_balance before solution 2
        users[id].balance = pre_balance;

        // solution 2:
        // I can shorten the notation passing by reference
        User storage sol2_user = users[id];
        sol2_user.balance = balance;
        assert(
            users[id].balance == balance &&
            users[id].balance != pre_balance // not really needed but more clear
        );
    }

    function getBalance(uint id) view public returns (uint) {
        return users[id].balance;
    }
}
1 Like
function updateBalance(uint id, uint balance) public {
         uint previousBalance = users[id].balance;
         User storage user = users[id];
         user.balance += balance;
         assert(user.balance == previousBalance + balance);
    }
1 Like

The 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

Nice solution @Waiguru :ok_hand:

Just be careful with your upper v lower case letters …

This will generate a compiler error, and prevent you from being able to deploy your contract.
You need to write the first letter of pragma with a lower case p

Nice solution @Cosmin_Fechet :ok_hand:

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 does make more sense to do what you’ve done, and add an amount to the user’s existing balance using the addition assignment operator += (instead of the assignment operator = ).

However, if we add to , instead of replacing , the existing balance, I think the code looks 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 (to give a new total balance), rather than replacing it with a new balance .

There is always more than one solution to these assignments, and alternative interpretations are equally valid.


Assert statement

This is mostly well-coded. The only thing I would do differently is reference the new balance in the actual mapping, rather than in the local storage variable user (the storage pointer) …

assert(users[id].balance == previousBalance + balance);

We want to check that the invariance holds true (i.e. the end result of our updateBalance function) in its final and persistent “resting place”, which in this case is in the mapping, because this is where any error would have the most impact.

Just let me know if you have any questions :slight_smile:

1 Like

Thanks Jon. Your feedback is very much appreciated. I updated the function.

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

Nice solutions @fedev :ok_hand:

A couple of comments …

(1)

I would say that your solution 1 is shorter and more concise than your solution 2 and, in this particular example, also clearer. However, sometimes using a local storage variable as a pointer can be a useful intermediate step when performing more complex operations involving multiple properties and their values: it can help us to set our code out into separate logical steps, making it easier to read and understand.

(2)

As we are replacing the balance with a new balance, rather than adding an amount to it, then I think this condition/constraint does actually make sense. However, because what it’s effectively doing is validating an input, and not checking an invariance, it should be placed in a require statement, positioned at the beginning of the function body, and modified to account for this change of position. This then also means that you don’t need to include and reference the additional local variable pre_balance, because the balance hasn’t been changed yet e.g. (based on your solution 1)

require(balance != users[id].balance, "Updating with same balance"); 
users[id].balance = balance;
assert(users[id].balance == balance);

If an input value needs to be checked or validated with a require() statement, then this should normally be the first line of code in the function body to be executed. If require() fails, then the transaction will revert wherever it is placed; but any code placed before require(), and which will already have been executed before require() triggers revert, will result in less gas being refunded than if the check had been the first operation to be performed within the function body. This is because only the gas cost associated with the unexecuted operations after require() is saved; any gas already consumed executing code for the operations before and including require() is not refunded. You can probably see, therefore, that the earlier in the function require() is placed, the less the cost of gas will be if it throws and the transaction reverts.

Let me know if anything is unclear, or if you have any questions :slight_smile:

Looking good @Cosmin_Fechet :ok_hand:

Nicely modified!

1 Like

Hey @jon_m thank you!
Acknowledged both comments!

About the 1st I was aware: in this case it would be not worth, but it could be worth shortening the notation if we have a bigger function body with a lot of use of that storage location

About the 2nd definitely you’re right, I coded it more like a theoretical exploration that an actual function. The first lines are the original of the not-working example. But I get your point!!

1 Like
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 {
         // directly access the mapping via the index, and set balance
         users[id].balance = balance;
    }

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

}
1 Like