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

Code before:
					*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),
		}
	}

	// Randomly choose a winning ticket from among the total number of tickets.
	// Returns `None` if there are no tickets.
	fn choose_ticket(total: u32) -> Option<u32> {
		if total == 0 {
			return None
		}
		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() {
			if random_number < u32::MAX - u32::MAX % total {
				break
			}

			random_number = Self::generate_random_number(i);
		}

		Some(random_number % total)

Review comment (suggestion):
```suggestion
	/// 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.
```

Code after:
					*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),
		}
	}

	/// Randomly choose a winning ticket from among the total number of tickets.
	/// Returns `None` if there are no tickets.
	fn choose_ticket(total: u32) -> Option<u32> {
		if total == 0 {
			return None
		}
		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() {
			if random_number < u32::MAX - u32::MAX % total {
				break
			}

			random_number = Self::generate_random_number(i);
		}

		Some(random_number % total)