Hi @bjorkmanders,
Nice solution 
And welcome to the forum — good to see you in here! I hope you’re enjoying this course. If you have any questions or are unsure about anything, check out the discussion threads here as you’ll find lots of useful information. But if you can’t find what you’re looking for, then please post your question and we’ll do our best to help you out 
pragma solidity 0.5.1;
// cursus Smartcontracts
// 10 sept 2020
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]; // original line code
// First solution is to update from volatile memory to permanent storage
User storage user = users[id];
user.balance = balance;
// better Second solution is to select directly the record of the mapping with [id] and Update the balance
// users[id].balance = balance ;
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
I suggest 2 solutions, first is to change memory to storage but this will consume permanent storage
2e solution is to use the mapping directly
See code for implementation
This happens because when a variable with a storage data location type is later assigned to memory, this creates duplicate copies of the data and whenever the user makes a change or an update, the update is written within the memory copy instead of storage, and thus, does not show up in the original storage copy … making the update not visible.
After independent research, I learned that this problem can be solved by replacing memory with “storage” within the updateBalance function code as shown below and 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;
}
}
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;
}
}
Changed “memory” to “storage” in the updateBalance function.
Hi @RLion,
Both of the solutions you have highlighted are correct
In actual fact, this first solution hardly uses any more gas than the second solution:
users[id].balance = balance;
And when optimization is enabled, there is no difference in gas cost.
I think this is because the line:
effectively makes user a direct reference (a “pointer”) to the User instance (with that ID) stored permanently in the mapping at users[id]. So the second line…
is effectively the same as…
However, I do think that the second solution has the advantage of being more concise.
Hi @NeoKanoPheus,
Nice solution and research 
And welcome to the forum — good to see you in here! I hope you’re enjoying this course. If you have any questions or are unsure about anything, check out the discussion threads here as you’ll find lots of useful information. But if you can’t find what you’re looking for, then please post your question and we’ll do our best to help you out
… unless of course the value stored locally in memory is assigned to the state variable or mapping in permanent storage, before it is lost when the function finishes executing. This would be an alternative solution, although a much less efficient and more costly one in terms of gas.
Yes, of course. Thank you and will do!
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;
users[id].balance = balance;
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
Seems like the simplest solution is to change the data location for the updateBalance function from memory --> storage.
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;
}
}
However, as the storage data location is the most gas intensive, there may be a more optimal solution.
Playing around with the contract and I realized that since we can also return updateBalance directly to the Users mapping.
However, since addUser and updateBalance are both saving the inputs directly to the mapping, it seems inefficient to have both, so we can also delete the addUser function.
Filip, is there any benefit to keeping both addUser function and update Balance function? It seems inefficient to keep both if we change updateBalance to include the ID mapping as well:
pragma solidity 0.5.1;
contract MemoryAndStorage {
mapping(uint => User) users;
struct User{
uint id;
uint balance;
}
function updateBalance(uint id, uint balance) public {
// User storage user = users[id];
//user.balance = balance;
users[id].id = id;
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 {
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 {
User storage user = users[id];
user.balance = balance;
}
function getBalance(uint id) view public returns (uint) {
return users[id].balance;
}
}
To be honest, this solution is one of the most optimal in terms of gas cost. By changing memory to storage in the line…
… you are only creating a reference (like a “pointer”) to a User instance in the users mapping — the User instance with the [id] . So you aren’t actually declaring a separate variable in storage. In terms of gas cost, your solution is only very slightly higher than:
function updateBalance(uint id, uint balance) public {
users[id].balance = balance;
}
In fact, when optimization is enabled, the gas costs are the same. I imagine that’s because they are effectively doing exactly the same thing, just expressing it with different code.
Well spotted, @sharkycryptoninja!
However, this duplication of functionality only happens because our contract example is extremely simple, for illustration purposes. In reality, we wouldn’t want the updateBalance function to also be able to create new balances (where the user didn’t previously exist). This should be restricted to the addUser function. And in turn, the addUser function should only be able to create new user balances, and not overwrite existing ones.
Other students have suggested different ways to introduce this restriction, and separation of functionality. By doing this, they’ve made the contract more meaningful. As you have already started to think more deeply about the issues involved with this assignment, perhaps you would find it helpful to have a go at doing this as well. If you need some inspiration you’ll find their suggestions, and our comments and feedback, within this discussion thread.
Hi Filip,
For original function updateBalance(uint id, uint balance) public compiler suggested adding view in the function header as it didn’t change anything, which makes it even easier to fix it.
Solution 1
function updateBalance(uint id, uint balance) public {
User memory user = users[id];
user.balance = balance;
// save local user to mapping users
users[id] = user;
}
Solution 2
function updateBalance(uint id, uint balance) public {
// change memory to storage
User storage user = users[id];
user.balance = balance;
Hi @Irek,
Your two alternative solutions are correct
The warning that suggests adding view is because the initial function doesn’t change the contract’s state. However, if we add view, while the warning disappears, this doesn’t solve our issue, because changing the contract’s state is precisely what we want to do i.e. we need the updated balance to be stored in the mapping and not get lost when the function finishes executing. When you modify the function for either of the 2 solutions you’ve posted, the warning disappears because now the function will change the contract’s state, and so it can no longer be restricted to view.
I hope that clarifies the warning for you and why we don’t want to add view .
hmm… is this right ? Now I can chance the balance 
function updateBalance(uint id, uint balance) public {
User storage user = users[id];
user.balance = balance;
}