First Flight #2: Puppy Raffle

Summary

This code is set up to run one lottery at a time in a set timeframe. It would be better to create an enum with states for open and closed so that it was clear to everyone which drawing they are entering and what phase the lottery is in.

Impact

It won't be clear to players which lottery they are entering
Also, the only limitation on selectWinner is that it can't be called before raffleDuration has passed since the last lottery but it can be called any time after that (which could be a long time or a short time). Players won't know whether the lottery is over, and players can still enter after raffleDuration has passed which is probably not what the other players would expect. If someone calls enterRaffle after raffleDuration has passed but selectWinner hasn't been called yet, then that player would be entered into the current raffle when other players would expect them to be in the next lottery.

Tools Used

Manual review

Recommendations

Make the following changes. Also, the good thing about this approach is that, as long as the owner calls changeLotteryState soon after raffleDuration has passed, then you will also enforce raffleDuration as the length of each raffle without having to add a check to enterRaffle that raffleDuration hasn't passed.

Add an enum:

Add a variable raffleState:

Set raffleState to OPEN in constructor:

Add a function for owner to call to place the lottery in calculating state (which owner should do once raffleDuration has passed since the last lottery):

Add this check to the beginning of enterRaffle to prevent people from entering the lottery when it is not open:

Add a line at the end of selectWinner to set the state back to open: