2

I need to loop though all of the members of a map and change their property (which is a struct).

the way i'm doing this is when adding a member to the map object also save it's address inside an array and later loop through the array and load the user's struct from the map then do changes.

struct Users {
    address user;
    uint balance;
    uint bonus;
}

mapping (address => Users) instructors;
address[] public allUsers;

function setInstructor(address _address, uint balance, uint _bonus) onlyOwner public {
    Users storage instructor = instructors[_address];

    instructor.user = _address;
    instructor.balance = _balance;
    instructor.bonus = _bonus;

    allUsers.push(_address) -1;
}

function addBalance(_amount) onlyOwner public {

    uint length = allUsers.length;

    for (uint i = 0; i < length ; i++) {
        Users[allUsers[i]].balance = Users[allUsers[i]].balance + (Users[allUsers[i]].bonus * _amount);
    }
}

I was wondering if there is a better way to this? because user's can go pretty a lot and a for loop is expensive in terms of gas usage.

should i user a external server to do the loop?

5
  • The pattern x.push(y) -1 seems to be a pretty solid indication of users who don't quite understand what they're doing. Do you understand what you're doing? Commented Nov 15, 2019 at 19:36
  • @goodvibration yea that part tbh i'm not sure what it's for i just got it from somewhere as an example code for my question. but that is not my problem, i'm asking if there is a better way than doing a loop? or else i'd have to do the loop in an external server i guess.
    – Adrin
    Commented Nov 15, 2019 at 19:48
  • What makes you think that would cost less? In fact, it would likely cost more. Commented Nov 15, 2019 at 19:53
  • I think that you should simply add a state-variable uint amount, which you increment in function addBalance. Then, implement function getBalance(address _user) which will return Users[_user].balane + Users[_user].bonus * amount. Commented Nov 15, 2019 at 19:58
  • @goodvibration can't do that because [_user].bonus can be changed by user at anytime.
    – Adrin
    Commented Nov 15, 2019 at 21:00

2 Answers 2

4

Trust me. Everytime (everytime!) you are thinking to loop trough a dataset whose dimensions you cannot predict at the “start of the time“ in a blockchain based system, you are using blockchain for the wrong thing and/or your algorithm must be changed by rethinking it.

Your model should be something where if a new “user” is added, the simply operation to add it changes all what is needed to have the same needed property of the set restored.

This way only you can cope with an unlimited number of “user” addition without infinite computational power available.

The sequential way of thinking (to sort, to use “for loops” to move trough the dataset and so on) are from a different paradigm: the sequential machine. It is not the case on blockchain based systems. Here they can have a very much limited role only.

I hope this can help you in solving your problem.

ADDED after your comment:

I want give you an example of paradigm (not an example of solution, I cannot know your application). Try to to think to maintain some data structure which tracks of the operations you have to do for all the balances in order to update them and that you update the balance of the single user in a special instant, f.i. when he makes some actions, for instance when he queries in order to know his updated balance. Can be true that nobody be interested in having all the values updated in the blockchain if not,f.i., when the blockchain is queried by someone. You should, f.i., maintain a certain record of the data needed to calculate the balance starting from the state variables and to apply the calculation once, when the user asks for his balance. This works if you are able to make a little vector of updating values able to represent the situation and to update the relevant data using it. In other words: if you need to have an operation (let’s call it “OP”) which instantly update your single data record, it is absolutely equivalent to loop trough all the single data record updating all of them applying the OP one by one until finished cancelling the information which defines the OP, OR to maintain stored both single data record and the OP and to apply the OP when needed only. The first is the typical FORTRAN like coding, the second is a typical blockchain approach. The first require s you to have a for loop on all the users and a for loop on all the properties of the OP, the second only need you apply the for loop on the properties expressed by OP.

I hope this can help too!

2
  • thanks. yea i'm doing this now: when there is a new user contract fires an event and a server will catch it and save the new user's address in a database. then every time i need to add balance to users i read their data from the contract and do the calculation and the loop in the server and at the end i send the result back to the contract.. that is the best way i could think of
    – Adrin
    Commented Nov 16, 2019 at 6:41
  • I added a possibly useful example in the answer (it was too long to be inserted as comment).
    – Rick Park
    Commented Nov 16, 2019 at 9:58
3

The pattern you are using is valid for certain table-like sets, but you should be aware of the limitations.

It is ill-advised to do anything with a contract that loops - Getting Loopy with Solidity.

You can, however, make it possible for a client to loop. It should be understood that "client" is an off-chain entity, in this context, because if it wasn't then it would doing something loopy.

struct Users {
    // address user; <=== not needed
    uint balance;
    uint bonus;
}

mapping (address => Users) instructors;
address[] public allUsers;

function userCount() public view returns(uint) {
    return allUsers.length;
}

function userAtIndex(uint row) public view returns(address, uint, uint) {
    address userAddr = allUsers[row];
    Users storage u = instructors[userAddr];
    return (userAddr, u.balance, u.bonus);

You don't need the address in the struct, because to get there you would have to find it in the mapping. If you can do that, you already know. userCount() is tell a client how many rows to iterate because discovering by trapping an error (when walking out of bounds) seems like bad form. It's not strictly necessary.

Please pardon my typos if I botched the example. I haven't compiled it.

You said you want to update a property for everything. Consider a strategy such as writing it in one place. Also consider that, in theory, if a value is computable in a code block inside a loop then it might be computable on-the-fly. Writes themselves are extraordinarily expense operations in Ethereum so you need to economize on those.

allUsers.push(_address) -1; returns the row in the array the value was just pushed to. This is useful in systems that rely on pointers. An example is the CRUD pattern that uses it to enable delete. You are not using it for anything (uint row = allUsers.push(_address) -1;), so allUsers.push(_address); will suffice. It's not broken, the -1 is just extraneous.

Hope it helps.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.