-4
\$\begingroup\$

I have this code but it brings repeated results. I want it to generate without repeating the letters

private char[] lettersArr = "abcdefghijklmnopqrstuvwxyz".ToCharArray();

public char generateLetter()
{
    return lettersArr[rdm.Next(lettersArr.Length)];
}

The "L" is independent Result from my code{Lw, Lk, Lk, La} Results i want {La. Lb. Lc, Ld}

\$\endgroup\$

closed as off-topic by Pieter Witvoet, Martin R, BCdotWEB, AlexV, dfhwze Jul 17 at 16:11

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review." – Pieter Witvoet, Martin R, BCdotWEB, AlexV, dfhwze
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Have a look at the Fisher–Yates shuffle algorithm. But of course, if you call the method more than 26 times, you will inevitably have repeating letters. \$\endgroup\$ – Olivier Jacot-Descombes Jul 17 at 14:59
  • \$\begingroup\$ Just to confirm your expectations, abababababababab would be a valid output since it doesn't have repeating letters, right? Or are you expecting for each letter to be used in the same ratio (e.g. a 52 letter string contains every letter exactly twice)? It helps to disambiguate exactly what you're expecting (examples help a lot!). Especially with randomness, different people have different expectations and it's hard to infer your expectation from the current question. \$\endgroup\$ – Flater Jul 17 at 14:59
  • 2
    \$\begingroup\$ If you're looking for help with implementing a new feature, then Code Review is not the right place - here we only review code that is already working as intended. Stack Overflow would be a more appropriate place to ask. \$\endgroup\$ – Pieter Witvoet Jul 17 at 15:00
  • \$\begingroup\$ i added an example @Flater \$\endgroup\$ – Alexander Jul 17 at 15:16
  • \$\begingroup\$ We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review. Please follow the tour, and read "What topics can I ask about here?", "How do I ask a good question?" and "What types of questions should I avoid asking?". \$\endgroup\$ – BCdotWEB Jul 17 at 15:49
-1
\$\begingroup\$

I'll use a deck of cards as an example here.

What your current code does is draw a random card from a deck. Then, the second time, it draws a random card from a complete deck, which means that there's a change you draw the same card twice. The problem persists for all subsequent draws.

What you want to do is when you draw a second card is that you want to draw it from the same deck, i.e. the 51 card deck (because you already drew a card earlier).

In a deck of cards, when you draw a random card, you inherently remove it from the deck. But that's not how it works in code. You copy a random value from the array, but you don't automatically remove it from the array.

There are two ways of doing this.

1. Remove the drawn card from the deck before drawing the next card.

In other words, when you get a random letter, you remove that letter from lettersArr so that you can't get the same letter again.

string remainingLetters = "abcdefghijklmnopqrstuvwxyz";

// draw the first "card"
var firstLetter = remainingLetters[rdm.Next(remainingLetters.Length)];

// remove it from the "deck"
remainingLetters = remainingLetters.Replace(firstLetter, String.Empty);

// draw the second "card"
var secondLetter = remainingLetters[rdm.Next(remainingLetters.Length)];$

This can of course be improved by an iterative approach. Also, beware that this makes it impossible for you to get more than 26 letters as your deck will be empty.

There are more performant variation of this algorithm, but this is just an example. I'm skipping the optimalizations because I suggest using the second approach as I find it far better to use:

2. Shuffle the deck

What I mean by shuffling the deck is that you (randomly) rearrange the letters rather than extracting them from the array one by one. This is very analogous to what you'd do to a deck of cards, and it makes a lot more intuitive sense.

I much prefer this method because it's so much cleaner, and you can use LINQ

var orderedAlphabet = "abcdefghijklmnopqrstuvwxyz".ToCharArray();

var shuffledAlphabet = orderedAlphabet.OrderBy(letter => rdm.Next()).ToArray();

Basically, LINQ will generate a random number (rdm.Next()) for every letter, and will then sort the letters by the numerical value of the random number.

The end result is a string (or character array) with the exact same 26 letters, but the order is randomized.

If you then want to fetch letters from the randomized array, just read from left to right. You can either use a substring to get a chunk of a certain size all at once:

var chunk = shuffledAlphabet.Substring(0,5); // 5 letters

or read it letter by letter using a counter that you increment:

private int counter = 0;

private char GetNextLetter()
{
    return shuffledAlphabet[counter++];
}

Don't forget to check your upper bound too!

Or if you need all 26, you don't need to process it any further and can just return shuffledAlphabet as is.

private char[] GetShuffledAlphabet()
{
    return shuffledAlphabet;
}
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.