Welcome to Code Review Stack Exchange
Code Review is a question and answer site for seeking peer review of your code. It's built and run by you as part of the Stack Exchange network of Q&A sites. We're working together to improve the skills of programmers worldwide by taking working code and making it better.
Before posting a question, please read What topics can I ask about here? and How do I ask a good question?.
To get you the best answers, we’ve provided some guidance:
| 1. Code must produce the desired output |
| 2. Code must not error or contain known bugs |
| 3. Code must be written or maintained by you. |
| 4. Titles should describe what the code does |
| 5. The more code you provide the more we can help |
| 6. After an answer is posted, you must not edit your question to invalidate any advice. |
We're a little bit different from other sites. Here's how:
Ask questions, get answers, no distractions
This site is all about getting answers. It's not a discussion forum. There's no chit-chat.
Just questions...
...and answers.
Good answers are voted up and rise to the top.
The best answers show up first so that they are always easy to find.
The person who asked can mark one answer as "accepted".
Accepting doesn't mean it's the best answer, it just means that it worked for the person who asked.
Generate random numbers without repetitions
I want to generate a list of N different random numbers:
public static List<int> GetRandomNumbers(int count)
{
List<int> randomNumbers = new List<int>();
for (int i=0; i<count; i++)
{
int number;
do number = random.Next();
while (randomNumbers.Contains(number));
randomNumbers.Add(number);
}
return randomNumbers;
}
But I feel there is a better way. This do...while loop especially makes this ugly. Any suggestions on how to improve this?
2,4344 gold badges21 silver badges35 bronze badges
2 Answers
The algorithm looks fine to me. It shows no clear flaws, and will work fine in common use cases.
For extreme cases of selecting most of the numbers in a very large interval, it will be very inefficient. But unless that is an expected use case, I would not bother changing the code to more complicated approaches.
But I suggest you always use explicit braces around blocks even when they have a single statement:
do
{
bar();
}
while (foo);
I would also rename the method to GetNRandomNumbers, to make the method's name match its purpose better.
2,88216 silver badges29 bronze badges
Another solution is to use a Format Preserving Encryption cipher (such as AES using the FFX mode), configured to map from 32 bit integers to 32 bit integers.
Seed it randomly, then simply encrypt the first 'count' integers.
These encrypted numbers will be as random as the seed, and won't repeat.
1911 bronze badge
Get answers to practical, detailed questions
Focus on questions about an actual problem you have faced. Include details about what you have tried and exactly what you are trying to do.
Not all questions work well in our format. Avoid questions that are primarily opinion-based, or that are likely to generate discussion rather than answers.
Questions that need improvement may be closed until someone fixes them.
Tags make it easy to find interesting questions
All questions are tagged with their subject areas. Each can have up to 5 tags, since a question might be related to several subjects.
Click any tag to see a list of questions with that tag, or go to the tag list to browse for topics that interest you.
Generate random numbers without repetitions
I want to generate a list of N different random numbers:
public static List<int> GetRandomNumbers(int count)
{
List<int> randomNumbers = new List<int>();
for (int i=0; i<count; i++)
{
int number;
do number = random.Next();
while (randomNumbers.Contains(number));
randomNumbers.Add(number);
}
return randomNumbers;
}
But I feel there is a better way. This do...while loop especially makes this ugly. Any suggestions on how to improve this?
2,4344 gold badges21 silver badges35 bronze badges
You earn reputation when people vote on your posts
Your reputation score goes up when others vote up your questions, answers and edits.
As you earn reputation, you'll unlock new privileges like the ability to vote, comment, and even edit other people's posts.
At the highest levels, you'll have access to special moderation tools. You'll be able to work alongside our community moderators to keep the site focused and helpful.
Improve posts by editing or commenting
Our goal is to have the best answers to every question, so if you see questions or answers that can be improved, you can edit them.
Use edits to fix mistakes, improve formatting, or clarify the meaning of a post.
Use comments to ask for more information or clarify a question or answer.
You can always comment on your own questions and answers. Once you earn 50 reputation, you can comment on anybody's post.
Remember: we're all here to learn, so be friendly and helpful!
The algorithm looks fine to me. It shows no clear flaws, and will work fine in common use cases.
For extreme cases of selecting most of the numbers in a very large interval, it will be very inefficient. But unless that is an expected use case, I would not bother changing the code to more complicated approaches.
But I suggest you always use explicit braces around blocks even when they have a single statement:
do
{
bar();
}
while (foo);
I would also rename the method to GetNRandomNumbers, to make the method's name match its purpose better.
2,88216 silver badges29 bronze badges
Unlock badges for special achievements
Sign up to get started
Signing up allows you to:
- Earn reputation when you help others with questions, answers and edits.
- Select favorite tags to customize your home page.
- Claim your first badge: Informed
Code Review Stack Exchange is part of the Stack Exchange network
Like this site? Stack Exchange is a network of 182 Q&A sites just like it. Check out the full list of sites.
