Data Location Assignment [OLD]

So I just watched the Data Location Assignment solution, and I see that the solution was more concise than replacing memory with storage. I understand that the new balance will automatically be stored into storage simply by assigning it to the state variable mapping because that’s already in storage.

However, changing memory to storage seems to function the same way. Is there a possible issue if we were to continue building on top of this solution? Or is this just a less concise alternative? Is it perhaps more expensive written this way?

1 Like

This error happens because we take a copy of what is in our mapping, change the copy and not assign it back to our mapping.

I also removed the id field in User because the uint in our mapping does the job already
and I changed uint in mapping to the address of the sender.

pragma solidity 0.5.1;
contract MemoryAndStorage {

mapping(address => User) users;

struct User{
    uint balance;
}

function addUser(uint balance) public {
    users[msg.sender] = User(balance);
}

function updateBalance(uint balance) public {
     users[msg.sender].balance = balance;
}

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

}

2 Likes

I think that the updateBalance function record the new balance in memory and not in storage. As soon the function is finished the value will be gone.

In the updateBalance function:

function updateBalance(uint id, uint balance) public {
     User storage user = users[id];
     user.balance = balance;
}
We should use storage instead of memory in order to save the balance after the function is executed. Memory gets delted as soon as we finish the function call.
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;
}

}

I updated the pragma from 0.5.1; to 0.5.12; as 0.5.1 was giving me compiler errors on my version of chrome.

Then, I commented out the user memory allocation and simply set the users[id].balance variable to the balance specified.

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

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

By adding the last line of code, our mapping “users” also gets permanently updated (storage).

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

Data Location Assignment
Little changes in the updateBalance function. The new balance value should be stored as storage value, not in memory.

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

There were a couple problems:

  1. In the “updateBalance” function the new balance was being assigned to a local var in memory and not the users property in storage. I’m guessing when you do User memory user = users[id]; this creates a COPY of the real values in storage. So when the var in local memory is modified it’s a separate instance and has no effect on the value in storage. This is clear if you add an assert statement to the original assignment code to check the balance. The CS term for this would “pass by value” and NOT “pass by reference”.

  2. The “updateBalance” function did not check to see if the user id has already been created. A new User object is auto created with id=0 in this case.

  3. Just for kicks I restricted the create and update functions to the contract owner using modifiers, and added an array to keep track of the added user id’s as this was mentioned as a good design pattern to follow.

pragma solidity 0.5.12;
contract MemoryAndStorage {

    mapping(uint => User) users;
    uint[] private userIds;
    
    address owner;

    struct User{
        uint id;
        uint balance;
    }
    
    modifier onlyOwner() {
        require(msg.sender == owner);
        _;
    }
    
    constructor() public {
        owner = msg.sender;
    }

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

    function updateBalance(uint id, uint balance) public onlyOwner {
         require(users[id].id == id, "User ID does not exist");
         
         users[id].balance = balance;
         assert(users[id].balance == balance);
    }

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

}
3 Likes

I commented the script and the changes i did to the original script.

I made the mapping private.
I also changed the names of the arguments to be more logical to me, and then i understood that the my updated balance was stored to memory and would be deleted when the function call ended. then i removed the line (User memory user = users[id]; ) and set the users[id].balance to newBalance instead.
I also set the functions to view, just because the compiler told me to do so. :wink:

I hope you understood what i did and that the assignment is passed. At least it workes…

pragma solidity 0.5.12;
// updated from 0.5.1 to 0.5.12 compiler


contract MemoryAndStorage {
    
    mapping(uint => User) private users;
    // Creates an mapping the key is a number and the value is User, mapping is private and is named users

    struct User {
    // Creates an struct User and contains an id number and a balance
        uint id; 
        uint balance;
    }

    function addUser(uint id, uint initialBalance) public {
    // This function add User and takes id and initialBalance as arguments
        users[id] = User(id, initialBalance);
        // this creates an User with an id and initialBalance, then adds it to the users mapping
    }

    function updateBalance(uint id, uint newBalance) public {
    // This function updates the balance of the id to the newBalance argument 
        users[id].balance = newBalance;
    }

    function getBalance(uint id) public view returns(uint) {
    // This function returns the stored value(balance) in the users mapping of the given id argument
        return users[id].balance;
    }

}

Now i`m going to watch the assignment solution… :slight_smile:

As a career maintenance programmer I’m inclined to try to determine what the existing code does, extrapolate what it was trying to do, and only then minimally code the gap. I typically leave a thick trail of comment breadcrumbs in my wake.

    function updateBalance(uint id, uint balance) public {
        // Problem: update is executing but has no affect on balance
        User memory user = users[id];  // Copies mapped user to a memory User struct
        user.balance = balance; // Updates balance of memory User struct.
        
        // fails to push memory User struct to mapping in storage.
        users[id] = user; // <-- should fix it, if overwrites are allowed.
        
        // Note: User.id is superfluous to users[id]
    }

The remix debugger gave me a hint to add view in the addBalance function, which made me thinking about persistence off this storage. I changed memory to storage and it worked.
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;
}

}

In the case, change the way to change the memory from memory to storage, since the memory deletes the stored data at the end of the function and storage keeps them.

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

}

pragma solidity 0.5.12;
contract MemoryAndStorage {

mapping(uint => User) users;

struct User{
    uint id;
    uint balance;
}

address public owner;

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

constructor() public{
    
    owner = msg.sender;
}

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

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

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

}

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

}

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;
     //User memory user = users[id];
     //user.balance = balance;
}

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

}

I don’t understand the first solution where we change the memory to storage.
How the new value (User storage user) refers to (mapping(uint => User) users)
We change the user and then somehow affects the value users.

Oh… now I understand.
With the command User storage user = users[id]; the value user refers to the value users[id]. It isn’t like C++ that makes a new copy to the value user.

Correct me if I’m wrong.