Few questions about smart contract I created (just start learning)

I just started learning solidity and have some questiones about the smart contract I created for pratice/fun. Please let me know if any of my concepts are inaccurate, appreciate for all the advices and suggestiones.

Description:

the concept of this smart contract is really simple, whoever send the bigger amount of ether into the contract wins, it will pair you with whoever is before you (if no one is before you, you are player_one) and it will reset after 2 player are played (so it can be played again)

Code:

contract zero_one {

address public player_one;
address public player_two;
uint public player_one_amount;
uint public player_two_amount;


function zero_one() public{
    reset();
}

function play() public payable{
    //Scenario #1 Already have two player in game, dont accpet new player. do I even need this check at all? since smart contract execute serially i should never face this condition?
    if(player_one != address(0) && player_two != address(0)) throw;

    //Scenario #2 First player enter the game
    else if(player_one == address(0) && player_two == address(0)){
        player_one=msg.sender;
        player_one_amount = msg.value;
    }

    //Scenario #3 Second player join in, execute the game
    else{
        player_two = msg.sender;
        player_two_amount = msg.value;

        //check the amount send from player_one and player two,  whoever has the bigger amount win and get their money 
        if(player_two_amount>player_one_amount){
            player_one.transfer(player_one_amount+player_two_amount);
            reset();
        }
        else if(player_two_amount<player_one_amount){
            player_two.transfer(player_one_amount+player_two_amount);
            reset();
        }
        else{
            //return fund back to both player
            player_one.transfer(player_one_amount);
            player_two.transfer(player_two_amount);
            reset();
        }
    }
}

function reset() internal{
    player_one = address(0);
    player_two = address(0);
    player_one_amount = 0;
    player_two_amount = 0;
}
}

Questions

When sending ether back to users, do I need to calcualate how much gas is going to take? or would smart contract automatically deduct the gas from the amount i am going to send
Is it correct to set reset() as interal since i only want it to be called within the smart contract, it shouldn't be called by anyone else
Would Scenario#1 ever happen? since from what I understand smart contract do not have to worry about race condition and it should never be in that state?
1. Is adding playable to play correct? ( since user are going to send ether with this call)
Is using throw a bad practice? (warning on remix)
transfer vs send, why is it better to use transfer?
I coded this smart contract as a practice. I can already see that if someone want to game the system, he could just wait for someone to be player_one and check the amount player_one send(after the block is mined) and just send a bigger sum than that. Is there anyway this exploit could be stopped? Is there other security/flaws I did not see?

Thanks!

Solution

When sending ether back to users, do I need to calcualate how much gas is going to take? or would smart contract automatically deduct the gas from the amount i am going to send

The gas limit specified when executing a transaction needs to cover all activity end-to-end. This includes calls out to other contracts, transfers, etc. Any gas not used after the transaction is mined is returned to you.

Is it correct to set reset() as interal since i only want it to be called within the smart contract, it shouldn't be called by anyone else

internal is fine for what you're intending to do. But, it's more like protected access. A sub-contract can call internal methods. private provides the strictest visibility.

Would Scenario#1 ever happen? since from what I understand smart contract do not have to worry about race condition and it should never be in that state?

No, it should not happen. You are correct that transactions are processed serially, so you don't really have to worry about race conditions. That doesn't mean you shouldn't have this sort of protection in your code though...

Is adding playable to play correct? ( since user are going to send ether with this call)

Yes. Any method that is expecting to receive wei and uses msg.value needs to be marked as payable.

Is using throw a bad practice? (warning on remix)

throw is deprecated. You want to use one of revert, require, assert as of 0.4.13. The one you use depends on the type of check you're doing and if gas should be refunded. See this for more details.

transfer vs send, why is it better to use transfer?

send and transfer are similar. The difference is that send limited the amount of gas sent to the call, so if the receiving contract tried to execute any logic, it would most likely run out of gas and fail. In addition, failures during send would not propagate the error and simply return false. Source

I coded this smart contract as a practice. I can already see that if someone want to game the system, he could just wait for someone to be player_one and check the amount player_one send(after the block is mined) and just send a bigger sum than that. Is there anyway this exploit could be stopped? Is there other security/flaws I did not see?

Security is a much more in depth topic. Any data sent in a transaction is visible, so you can't hide the exploit you mentioned unless you use a private blockchain. You can encrypt data sent in a transaction yourself, but I don't believe you can encrypt sending ether.

In terms of other security issues, there are several tools out there that perform security checks on contracts. I'd suggest looking into one of those. Also, read through the security considerations page on the Solidity documentation.