Data Location Assignment

kasunkv · November 21, 2021, 7:33am

Assuming it is not specified that the new balance should be added to the already existing balance of the user. Here is my solution.

pragma solidity 0.8.9;

contract MemoryAndStorage {
    struct User {
        uint id;
        uint balance;
    }
    
    mapping(uint => User) users;
    
    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) public view returns (uint) {
        return users[_id].balance;
    }
}

giomiri · November 22, 2021, 11:12am

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

}

skplunkerin · November 24, 2021, 1:02pm

gist source

pragma solidity 0.8.7;

contract MemoryAndStorage {

    struct User{
        uint id;
        uint balance;
    }
    // state variable - using storage
    mapping(uint => User) Users;

    // PUBLIC
    
    // addUser creates a new user with the passed in _id and _balance,
    // adding them to the users state variable.
    function addUser(uint _id, uint _balance) public {
        // only add if the user doesn't exist
        require(!_userExists(_id), "user already exists, please use updateBalance()");
        Users[_id] = User(_id, _balance);
        _assertUserBalance(_id, _balance);
    }

    // updateBalance will modify a users balance if the passed in user
    // _id is found.
    function updateBalance(uint _id, uint _balance) public {
        _requireUserExists(_id);
        Users[_id].balance = _balance;
        _assertUserBalance(_id, _balance);
    }

    // getBalance returns the users balance if the passed in user _id
    // is found.
    function getBalance(uint _id) view public returns (uint) {
        _requireUserExists(_id);
        return Users[_id].balance;
    }
    
    // PRIVATE

    // _userExists verifies if the passed in user _id is found or not.
    function _userExists(uint _id) private view returns(bool) {
        User memory u = Users[_id];
        if (u.id == 0) {
            return false;
        }
        return true;
    }
    
    // _assertUserBalance verifies the user.balance matches the passed
    // in _balance.
    function _assertUserBalance(uint _id, uint _balance) private view {
        assert(_userExists(_id));
        assert(Users[_id].balance == _balance);
    }
    
    // _requireUserExists requires that the passed in user _id is found
    // in users.
    function _requireUserExists(uint _id) private view {
        require(_userExists(_id), "User doesn't exist, please create the user first.");
    }

}

Joeyj007 · November 25, 2021, 3:10am

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

}

jon_m · November 25, 2021, 5:45pm

Hi @skplunkerin,

Your solution works, including your additional functionality to ensure that…

the addUser function can only add users with IDs that don’t already exist;
the updateBalance function can only update the balances of existing users; and
the getBalance function can only be accessed with existing user IDs.

The additional code you’ve added demonstrates an excellent knowledge of the syntax already covered in this course I now encourage you to work at producing the same high quality functionality with more concise and cleaner code. This is an important consideration with smart contracts, because cleaner, more concise code generally results in a securer smart contract with less potential vulnerabilities or risk of bugs. With Ethereum smart contracts we also need to consider gas consumption, and cleaner, more concise code will usually reduce gas costs.

While it is good practice to place chunks of code which perform distinct operations and computations within their own separate functions, I think there is also a danger of overdoing this. For example, one of the advantages of using a require statement is that it combines the conditional logic of if/else statements with revert. It is already a concise piece of code, and so placing conditional logic in a separate function, and calling this function from within the require statement, when this condition can be coded more concisely within the require statement itself, is actually defeating the purpose of using a require statement in the first place. Separate helper functions are mostly useful where you have multiple require statements (or in your case assert statements), or more complex conditional logic. Where you have more than one function that needs the same require statement, the best way to avoid code duplication is to place the require statement within a modifier and add the modifier to the function headers.

You also have one situation that should be caught with a require statement, and not an assert statement. If addUser() is called with an ID of zero, the assert statement …

… fails … but for the wrong reason. This assert statement should fail if the user hasn’t been added correctly, but instead it is failing because a user with ID zero has been added. This is easily addressed by adding an additional require statement that prevents an input ID of zero. This is important, because your logic for checking whether a user exists or not is based upon Users[_id].id evaluating to zero only when the user doesn’t exist.

Have a look at my suggestions of how to address these issues in the following modified version of your solution…

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.7;

contract MemoryAndStorage {

    struct User{
        uint id;
        uint balance;
    }

    mapping(uint => User) users;
   
    modifier userExists(uint id) {
        require(users[id].id != 0, "User doesn't exist. Please, add one first.");
        _;
    }

    function addUser(uint _id, uint _balance) public {
        require(_id != 0, "ID cannot be zero"); 
        require(users[_id].id == 0, "User already exists. Please use updateBalance()");
        users[_id] = User(_id, _balance);
        _assertUserBalance(_id, _balance);
    }

    function updateBalance(uint _id, uint _balance) public userExists(_id) {
        users[_id].balance = _balance;
        _assertUserBalance(_id, _balance);
    }

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

    function _assertUserBalance(uint _id, uint _balance) private view {
        assert(users[_id].id != 0);
        assert(users[_id].balance == _balance);
    }

}

Let me know if you have any questions

skplunkerin · November 25, 2021, 6:09pm

Thank you for this! I really appreciate you taking the time to review my code and providing feedback with examples on how I can code better. I’m excited to review this later today when I can see your code next to mine to see exactly what’s changed, so I can connect your feedback to how it’s implemented.

angnimasherpa · November 28, 2021, 5:06pm

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 user = users[id]; // or replace both lines with users[id].balance = balance
         user.balance = balance;
    }

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

}

jon_m · November 29, 2021, 9:30pm

Hi @angnimasherpa,

users[id].balance = balance;

This is a correct solution, but if you remove memory and don’t replace it with storage …

… this won’t compile, because a local variable defined with a struct instance data-type (here, User) must have its data location explicity stated …

// Correct alternative solution
User storage user = users[id];
user.balance = balance;

Let me know if you have any questions

angnimasherpa · November 30, 2021, 6:49am

Ahh cool. Thanks for the correction here!

JaimeAS · November 30, 2021, 2:12pm

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

}

jon_m · November 30, 2021, 10:47pm

Nice solution @JaimeAS

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

mcs.olive · December 10, 2021, 1:34am

These are two possible solutions:

replacing memory with storage.
using in a more direct way: users[_id].balance = _balance;

I tried both with same results.
I prefer the second in this example, althoug in a more complex code, the first with a variable referencing to the user could be useful.

// SPDX-License-Identifier: GPL-3.0
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]; // replacing memory with storage 
         user.balance = _balance;
         // it could also be: users[_id].balance = _balance;
    }

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

}

zajiramu1062 · December 11, 2021, 3:08am

The two lines of the function get the specific User object out of the contract mapping-type variable “users” and assign it to the “user” variable (first line), which has temporary storage duration due to memory keyword in its declaration.

Then the balance property of this User type variable is updated by setting it to the input uint variable balance. Once the function finishes execution the “user” variable is destroyed and thus is never updated in the mapping variable “users”.

One way to fix this problem would be to replace the modified User type variable back into the contract mapping type variable “users” by adding the following line to the end of the function:

users[id] = user;

This is essentially modifying the contract variable “users” and this is ok since the function “updateBalance” is neither pure (does not read or write contract variables) nor is it view (reads from contract variables only).

Or one could be more succinct by re-writing the function as follows:

users[id].balance = balance;

This just indexes the User object using the id input, accesses its balance property and sets it to the balance input variable.

jon_m · December 11, 2021, 3:26am

Hi @mcs.olive,

This is a nice analysis of two alternative solutions

jon_m · December 11, 2021, 3:59am

Hi @zajiramu1062,

This is a very good explanation of the initial problem, and analysis of two alternative solutions You demonstrate a clear understanding of the Solidity syntax and smart contract issues that we have been studying so far

A couple of comments about terminology, which I hope you find helpful …

Rather than referring to this uint value as a variable, the correct term to use here is (function) parameter, or (input) argument.

Mappings are a type of state variable. It’s important to make a clear distinction between state variables, which store data persistently in the contract state, and local variables defined within functions, which only store values temporarily while the function is executing.

Just let me know if you have any questions

281Cypher · December 11, 2021, 8:22pm

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

}

cosimokryptos · December 12, 2021, 4:35am

Hey everyone a lot of theseanswers are going over my head I added balance to newbalance in the update function

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 newbalance) public {
         users[id].balance = newbalance;
    }

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

}

nbkeaton · December 12, 2021, 9:06pm

My solution for the memory assignment:

I changed ‘memory’ to ‘storage’ and included notes explaining each function


pragma solidity 0.7.5;
contract MemoryAndStorage {

	mapping(uint => User) users; //creating a mapping
	
	struct User { //defining a 'User' class
		uint id;
		uint balance;
	}
	
	function addUser(uint id, uint balance) public { //adding a user with inputs of ID number and balance amount
		user[id] = User(id, balance)
	}
	
	function updateBalance(uint id, uint balance) public { //update the mapping with new user balance info, commit it to permanent storage
		User storage user = user[id];
		user.balance = balance;
	}
	
	function getBalance(uint id) view public returns (uint) { //show the balance of the user after input of a user ID number
		return users[id].balance;
	}

}

Let me know what improvements I can make!

ryan_n · December 13, 2021, 8:18am

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

}

jon_m · December 13, 2021, 5:19pm

Hi @cosimokryptos,

Your solution is correct

If you are struggling to understand any of the solutions, have a good read of some of the posts in the forum discussion topic for that assignment or section of the course. You’ll find a lot of useful feedback, answers and explanations other students have received. Then, if you still don’t understand something, post your question (with any associated code) here in the forum and we will help you