Distribute ether from contract address to accounts

Question

I've recently started to play around with solidity and have a question regarding my dapp. Basically, it's the betting site where people can guess the weather forecast (cloudy, sunny, etc) on a specific date. Ex: weather in seattle on March 10th, 2018. Let's say 5 people have bet (0.1 ether each) and one person guessed it right. The winner gets all the money from the rest of 4 people. Now, based on this scenario I assume each person has to send ether to contract address when he/she bets,

function setBettingInfo (string _forecast, string _date) payable returns (bool success) {    
    // ... codes

    address(this).transfer(msg.value); // send betting price to contract address
}

and the winners will be evenly distributed with ether from the contract address on March 11th, 2018 when the dapp calls a forecast api to find out what the weather was like on March 10th. Sending ethers winners, I'm trying to use

function transfer(string _forecast, string _date) returns (bool success) {
    // ... code to get the winners
    // Distribute ethers to winners
    for (uint i = 0; i < winners.length; i++) {
        if (contractBalance[address(this)] < value) throw;
        contractBalance[address(this)] -= value;
        winners.account.transfer(value);
    }    

    return true;
}

Just wondering if this is a correct approach.

1. send betting price to contract address
2. pull result from forecast api and access contract method to find out winners
3. calculate and evenly distribute ethers from contract to those winner accounts

Help is appreciated!

Answer 1

You should avoid any loops of unknown length. For example if there is one person that wins then the function will cost (made up number) 5000 gas to execute. If 1000 people all win, then the function could suddenly cost 1 million gas to execute.

A better approach to this would probably be to implement a claimWinnings() function that each winner can call once the results are released. That way the contract will only ever need to make one transfer at a time. And in claimWinnings() it would remove that person from the list of winners so that they cannot claim it a second time.

This also has the added benefit (for you, the contract owner) that you would not be having to pay gas fees every time a competition ends.