2011-01-25 13 views
6

Quindi, ho scritto un piccolo e, da quello che inizialmente ho pensato, un metodo semplice in C#. Questo metodo statico è pensato per essere usato come un semplice generatore di password suggestione, e il codice è simile al seguente:La mia funzione C# statica sta giocando con me ... totalmente strano!

public static string CreateRandomPassword(int outputLength, string source = "") 
{ 
    var output = string.Empty; 

    for (var i = 0; i < outputLength; i++) 
    { 
    var randomObj = new Random(); 
    output += source.Substring(randomObj.Next(source.Length), 1); 
    } 

    return output; 
} 

Io chiamo questa funzione come questa:

var randomPassword = StringHelper.CreateRandomPassword(5, "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"); 

Ora, questo metodo quasi sempre restituire stringhe casuali come "AAAAAA", "BBBBBB", "888888" ecc., dove ho pensato che dovrebbe restituire stringhe come "A8JK2A", "82mOK7" ecc.

Tuttavia, ed ecco la parte strana; Se metto un punto di interruzione e passo a capo di questa iterazione riga per riga, ottengo in cambio il tipo corretto di password. Nel 100% degli altri casi, quando Im non esegue il debug, mi dà schifo come "AAAAAA", "666666", ecc.

Com'è possibile? Ogni suggerimento è molto apprezzato! :-)

BTW, il mio sistema: Visual Studio 2010, C# 4.0, progetto RTM ASP.NET MVC 3 con server di sviluppo ASP.NET. Non ho testato questo codice in nessun altro ambiente.

+0

Non so se causa il problema, ma probabilmente non si desidera creare una nuova istanza di Random in ogni iterazione. Dovresti farlo una volta prima del ciclo. Il costruttore predefinito di Random utilizza Environment.TickCount come seme. Inoltre, il valore predefinito per la sorgente non ha molto senso poiché penso che la stringa vuota causerà un errore nella chiamata a Sottostringa. –

+0

Avete ragione, signor Putty! Inizialmente avevo messo la stringa "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890". Grazie! – CoderBang

risposta

15

Sposta la dichiarazione per randomObj all'esterno del ciclo. Quando esegui il debug, lo crea ogni volta con un nuovo seme, perché c'è una differenza di tempo sufficiente perché il seme sia diverso. Ma quando non esegui il debug, il tempo di seed è fondamentalmente lo stesso per ogni iterazione del ciclo, quindi ti dà lo stesso valore di avvio ogni volta.

E un minimo Nit - è una buona abitudine usare un StringBuilder piuttosto che una stringa per questo genere di cose, quindi non è necessario reinizializzare lo spazio di memoria ogni volta che si aggiunge un carattere alla stringa .

In altre parole, in questo modo:

public static string CreateRandomPassword(int outputLength, string source = "") 
{ 
    var output = new StringBuilder(); 
    var randomObj = new Random(); 
    for (var i = 0; i < outputLength; i++) 
    { 
    output.Append(source.Substring(randomObj.Next(source.Length), 1)); 
    } 
    return output.ToString(); 
} 
+0

Ah, questo lo spiega! Grazie mille Ken! :) – CoderBang

+0

Personalmente sarei andato a fare un char [] (invece di StringBuilder) e ad estrarre il char dal sorgente tramite l'indicizzatore, ma piuttosto simile. –

+2

+1. Solo un'altra nota, è quasi sempre meglio dichiarare l'oggetto casuale il meno possibile. Mi spingerei fino all'estrazione della dichiarazione in un campo statico. Se hai avuto due thread che hanno colpito questa funzione contemporaneamente, c'è la possibilità che tu spenda le stesse identiche password. – Rob

4

Il comportamento che stai vedendo è perché Random è basata sul tempo, e quando non sei il debug vola attraverso tutti e 5 iterazioni allo stesso momento (più o meno). Quindi stai chiedendo il primo numero casuale dallo stesso seme. Quando esegui il debug, è necessario un tempo sufficiente per ottenere un nuovo seed ogni volta.

spostare la dichiarazione di Random fuori del ciclo:

var randomObj = new Random(); 
for (var i = 0; i < outputLength; i++) 
{ 
    output += source.Substring(randomObj.Next(source.Length), 1); 
} 

Ora si sta andando avanti 5 passi da un seme casuale invece di muoversi 1 passo dallo stesso seme a caso 5 volte .

2

Si sta creando un'istanza di una nuova istanza di Random() su ogni iterazione attraverso il ciclo con un nuovo seme dipendente dal tempo. Data la granularità dell'orologio di sistema e la velocità delle moderne CPU, questo praticamente garantisce di riavviare ripetutamente la sequenza pseudo-casuale con lo stesso seme.

Prova qualcosa come il seguente, anche se sei single-threaded, è possibile omettere in modo sicuro il blocco():

private static Random randomBits = new Random() ; 
public static string CreateRandomPassword(int outputLength, string source = "") 
{ 
    StringBuilder sb = new StringBuilder(outputLength) ; 
    lock (randomBits) 
    { 
    while (sb.Length < outputLength) 
    { 
     sb.Append(randomBits.Next(source.Length) , 1) ; 
    } 
    } 
    return sb.ToString() ; 
} 

È istanziare solo l'RNG volta. Ognuno attinge bit dallo stesso RNG, quindi dovrebbe comportarsi molto più come una fonte di entropia.Se è necessaria la ripetibilità per il test, utilizzare l'overload del costruttore casuale che consente di fornire il seed. Stesso seme == stessa sequenza pseudo-casuale.

Problemi correlati