Please submit your solution for Data Location Assignment here.
You can use the “Preformatted Text” Button to encapsulate any kind of code you want to show.
function formatText(){
let words = "I'm a preformatted Text box, Please use me wisely!"
}
Hi guys,
Here is my solution to the assignment. I used assert to find that the logic for updating the balance isn’t correct, and then updated the += and data location to storage. Hope that’s one of the ways to solve the 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 {
uint previousBalance = users[id].balance;
User storage user = users[id];
user.balance += balance;
assert(user.balance == previousBalance + balance);
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
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] = User(id, balance); // simply assign a brand new User object
// todo maybe refactor rest of code to map id with balance instead of duplicating id logic
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
In the “UpdateBlance” function, we should use “storage” instead of “memory”
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;
}
}
First I changed the function to update users mapping with balance. It worked. Then I think I remembered that by default state variables are stored in storage so I tried and changed the memory key word to storage. Worked as well:
function updateBalance(uint id, uint balance) public {
User storage user = users[id];
user.balance = balance;
// this works too: users[id].balance = balance;
}
First i just switched memory to storage, and that did the trick, but that made the function just do the same thing basically that the add user function did. So i made this thingy, so it first checks if there is a balance of specific user, and if there is, then it updates it, otherwise it throws an error “no such user”.
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 {
require (users[id].balance != 0, "no such user");
User storage user = users[id];
user.balance = balance;
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
it’s primitive but seems to work ok.
My solution was to access directly the user balance from the mapping. Basically I removed the intermediary variable “user”. The resulting function “updateBalance” is now:
function updateBalance(uint id, uint balance) public {
users[id].balance = balance;
}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;
}
}
function updateBalance(uint id, uint balance) public {
User memory user = users[id];
user.balance = balance;
}
solution: User storage user = users[id];
replace ‘memory’ with ‘storage’
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]; // I replaced "memory" with "storage" here -- thinking structs can't be memory
user.balance = balance;
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
pragma solidity 0.7.5;
contract MemoryAndStorage {
mapping(uint => User) contractUsers;
struct User{
uint userId;
uint userBalance;
}
//////////////////////////////////////////////////////////////////////////////////
// Adding private function for security reasons
///////////////////////////////////////////////////////////////////////////////////
function _addUser(uint _userId, uint _userBalance) private { // Add user private call
contractUsers[_userId] = User(_userId,_userBalance);
}
function _getBalance(uint _userId) private returns (uint) { // Get balance user for internal calls
return contractUsers[_userId].userBalance;
}
//////////////////////////////////////////////////////////////////////////////////
// Adding public functions external interactions
///////////////////////////////////////////////////////////////////////////////////
function addUser(uint userId,uint userBalance) public {
require(userId != 0); // Require not zero user id
_addUser(userId,userBalance);
}
function updateBalance(uint id, uint newBalance) public {
// Sustain contract executions
User storage contractUsers = contractUsers[id]; // Added storage for perssistance
contractUsers.userBalance +=newBalance;
}
function getBalance(uint id) public returns (uint) {
return _getBalance(id);
}
}
Hi @evasilev,
Your basic solution is essentially correct:
One thing to note, though, is that we aren’t actually adding an amount to the existing balance, but updating/setting it to a new balance, and so you only need the assignment operator = and not the addition assignment operator +=
I must admit though, it would seem more logical and practical to do it your way, and therefore to also name the input parameter amount instead of balance.
If we do change the intended functionality to your more practical one (adding the amount to the existing balance, instead of replacing it with a new one), then we would also need to start the assignment with the line…
user.balance += balance;
// instead of
user.balance = balance;
… because whichever one we use, that isn’t the error you want assert() to check for. At the moment your assert statement is only failing initially because it is checking for something that the function wasn’t actually meant to do anyway. However, if we change the functionality to add an amount to the existing balance, then using an assert statement becomes relevant and a meaningful check. But, what you have at the moment will then evaluate to true with the new initial incorrect code (with memory) as well, because it is comparing the previous balance from the mapping with the local variable user, which isn’t lost until after the assert statement has executed (when the function finishes executing). You need to compare the previous balance with the balance in the mapping after it is supposed to have been updated in order to actually check whether it has been updated correctly in persistent storage rather than just in a temporary local variable (the original problem you want to check has been resolved).
So, instead of:
… you would need:
assert(users[id].balance == previousBalance + balance);
I hope that makes sense, but do let us know if anything is unclear, or if you have any questions 
User is declared as a state variable therefore it is automatically and permanently stored in the storage. With the memory keyword after User inside the updateBalance function, it created a new User in the memory and no longer referencing to the User declared as a state variable.
This 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 {
User storage user = users[id]; // changed memory to storage
user.balance = balance;
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
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 memory user = users[id];
user.balance = balance;
users[id] = user;
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
function updateBalance(uint id, uint balance) public {
// User memory user = users[id];
users[id].balance = balance;
}
That’s correct @deanb3, but don’t forget to actually add storage, after deleting memory 
Struct instances can be stored/located in memory or storage. It just depends how we want to use them. Storing them in memory isn’t wrong if we don’t need to store their values persistently e.g. if we just need them temporarily to perform some kind of operation within the function.
Hi @gkassaras,
You’ve put a lot of effort and thought into this, and it does work 
Just a few observations:
contractUsers
The compiler should have given you a warning about this in Remix. While this doesn’t prevent your contract from working, it would be better practice to use different names e.g.
User storage userInstance = contractUsers[id];
userInstance.userBalance = newBalance;
However, the fact that you chose to use the same name raises an interesting point. Changing the local variable’s data location to storage makes it a pointer to the mapping i.e. a direct reference and not a copy. This is often useful for clarity when assigning multiple values to more complex data structures. In this more straightforward scenario, by using the same name, you have highlighted the fact that the local variable is an unnecessary additional step, and that we can achieve exactly the same thing by “cutting out the middle man”:
contractUsers[id].userBalance = newBalance;
= and not the addition assignment operator += in the line:userInstance.userBalance = newBalance;userInstance.userBalance += newBalance;amount instead of newBalance , as this makes more sense.view keyword. This is appropriate because these functions only read the contract state, but don’t modify it. In Remix, doing this will also turn the call buttons blue for these functions, enabling you to see the output directly below the buttons, instead of having to open and scroll through the transaction data in the Remix terminal.Nice solution @zai
Just some comments about your explanation:
Here, User is not a state variable, it is a struct which defines a new data type (User). It acts like a template without any assigned value(s) until it is used to create a new instance. The new data type User can be used in a new variable declaration to create either a local variable or a state variable with a value of type User assigned to it.
In the updateBalance function, when we change the data location of the local variable user from memory to storage, we aren’t creating a state variable. The variable is still local, but it now acts as a pointer (a reference) to users[id] — the User instance with the key id in the users mapping.
In this assignment, the users mapping is acting as a state variable, its values stored persistently in storage.
So, in this statement…
instead of:
"… no longer referencing to the User declared as a state variable.
this should read something like:
"… no longer referencing the User instance (with ID of "input parameter id") saved persistently in storage in the users mapping.
I hope this makes sense, but do let us know if anything is unclear or if you have any questions
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 memory user = users[id];
user.balance = balance;
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
Hi @jon_m
You are correct the remix online compiler did give me warnings. The main reason I wrote the code this way is also for clarity! From my perspective:
User storage contractUsers = contractUsers[id];
contractUsers.userBalance +=newBalance;
More clear compared to:
User storage userInstance = contractUsers[id];
userInstance.userBalance = newBalance;
Gives more clarity in the purpose of the code, meaning we add extra lines to allow easier debugging and code auditing. What would be interesting is how the contract cost increases or decreases, depending on expressions.
But I am sure that after optimisation the lines will be replaced with the same byte codes.