Repository: paritytech/substrate
File: frame/lottery/src/lib.rs (Rust)

Code before:

	// Logic for buying a ticket.
	fn do_buy_ticket(caller: &T::AccountId, call: &<T as Config>::Call) -> DispatchResult {
		// Check the call is valid lottery
		let config = Lottery::<T>::get().ok_or(Error::<T>::NotConfigured)?;
		let block_number = frame_system::Pallet::<T>::block_number();
		ensure!(
			block_number < config.start.saturating_add(config.length),
			Error::<T>::AlreadyEnded
		);
		ensure!(T::ValidateCall::validate_call(call), Error::<T>::InvalidCall);
		let call_index = Self::call_to_index(call)?;
		let ticket_count = TicketsCount::<T>::get();
		let new_ticket_count = ticket_count.checked_add(1).ok_or(ArithmeticError::Overflow)?;
		// Try to update the participant status
		Participants::<T>::try_mutate(
			&caller,
			|(lottery_index, participating_calls)| -> DispatchResult {
				let index = LotteryIndex::<T>::get();
				// If lottery index doesn't match, then reset participating calls and index.
				if *lottery_index != index {
					*participating_calls = Default::default();
					*lottery_index = index;
				} else {
					// Check that user is not already participating under this call.
					ensure!(
						!participating_calls.iter().any(|c| call_index == *c),
						Error::<T>::AlreadyParticipating
					);
				}
				participating_calls.try_push(call_index).map_err(|_| Error::<T>::TooManyCalls)?;
				// Check user has enough funds and send it to the Lottery account.
				T::Currency::transfer(caller, &Self::account_id(), config.price, KeepAlive)?;
				// Create a new ticket.
				TicketsCount::<T>::put(new_ticket_count);
				Tickets::<T>::insert(ticket_count, caller.clone());
				Ok(())
			},
		)?;

		Self::deposit_event(Event::<T>::TicketBought { who: caller.clone(), call_index });

		Ok(())
	}

	// Randomly choose a winner from among the total number of participants.
	fn choose_winner(total: u32) -> u32 {
		let mut random_number = Self::generate_random_number(0);

		// Best effort attempt to remove bias from modulus operator.
		for i in 1..T::MaxGenerateRandom::get() {

Review comment (refactor):
Using a `BoundedSet` here would make matters easier, but I'm not sure if it is worth the refactor rn.

Code after:
	fn do_buy_ticket(caller: &T::AccountId, call: &<T as Config>::Call) -> DispatchResult {
		// Check the call is valid lottery
		let config = Lottery::<T>::get().ok_or(Error::<T>::NotConfigured)?;
		let block_number = frame_system::Pallet::<T>::block_number();
		ensure!(
			block_number < config.start.saturating_add(config.length),
			Error::<T>::AlreadyEnded
		);
		ensure!(T::ValidateCall::validate_call(call), Error::<T>::InvalidCall);
		let call_index = Self::call_to_index(call)?;
		let ticket_count = TicketsCount::<T>::get();
		let new_ticket_count = ticket_count.checked_add(1).ok_or(ArithmeticError::Overflow)?;
		// Try to update the participant status
		Participants::<T>::try_mutate(
			&caller,
			|(lottery_index, participating_calls)| -> DispatchResult {
				let index = LotteryIndex::<T>::get();
				// If lottery index doesn't match, then reset participating calls and index.
				if *lottery_index != index {
					*participating_calls = Default::default();
					*lottery_index = index;
				} else {
					// Check that user is not already participating under this call.
					ensure!(
						!participating_calls.iter().any(|c| call_index == *c),
						Error::<T>::AlreadyParticipating
					);
				}
				participating_calls.try_push(call_index).map_err(|_| Error::<T>::TooManyCalls)?;
				// Check user has enough funds and send it to the Lottery account.
				T::Currency::transfer(caller, &Self::account_id(), config.price, KeepAlive)?;
				// Create a new ticket.
				TicketsCount::<T>::put(new_ticket_count);
				Tickets::<T>::insert(ticket_count, caller.clone());
				Ok(())
			},
		)?;

		Self::deposit_event(Event::<T>::TicketBought { who: caller.clone(), call_index });

		Ok(())
	}

	/// Randomly choose a winning ticket and return the account that purchased it.
	/// The more tickets an account bought, the higher are its chances of winning.
	/// Returns `None` if there is no winner.
	fn choose_account() -> Option<T::AccountId> {
		match Self::choose_ticket(TicketsCount::<T>::get()) {
			None => None,
			Some(ticket) => Tickets::<T>::get(ticket),
		}