0

I am trying to shuffle a string array using JS. I hard-coded three sets of characters. 3 characters from each set were chosen at random and then concatenated. So the concatenated string is 3 letters, followed by 3 numbers, followed by 3 symbols. I want to shuffle this concatenated string so their order is randomized.

I already checked How to randomize (shuffle) a JavaScript array?, and my algorithm and code essentially matches what the second solution was, except for the fact that I have the shuffle loop in a bigger function (instead of as its own function). Perhaps I should add the shuffling to its own function and call it within the bigger function, but I think that will still give me the same result.

The "shuffling part" of the function isn't actually shuffling. When I debug it with console.log(temp) and console.log(tp[rnd]), the correct values are showing. Basically, the function is returning the unshuffled string that I wanted to shuffle.

var letterSet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
var numberSet = "0123456789";
var symbolSet = "~!@#$%^&*()-_+=><";

this.generatePassword = function() {
        var rl = "";
        var rn = "";
        var rs = "";
        var tp = "";

        // 3 random letters
        for(var i = 0; i < 3; i++) {
            var rnd = Math.floor((Math.random() * 52));
            rl += letterSet[rnd];
        }

        // 3 random numbers
        for(var i = 0; i < 3; i++) {
            var rnd = Math.floor((Math.random() * 10));
            rn += numberSet[rnd];
        }

        // 3 random symbols
        for(var i = 0; i < 3; i++) {
            var rnd = Math.floor((Math.random() * 17));
            rs += symbolSet[rnd];
        }

        // String concatenation
        tp = rl + rn + rs;

        // Shuffling part
        for(var i = 0; i < tp.length; i++) {
            var rnd = Math.floor(Math.random() * tp.length);
            var temp = tp[i];
            tp[i] = tp[rnd];
            tp[rnd] = temp;
        }

        return tp;
    }();

I don't understand what is going wrong.

Community
  • 1
  • 1
BrianRT
  • 1,952
  • 5
  • 18
  • 27
  • possible duplicate of [How to randomize (shuffle) a JavaScript array?](http://stackoverflow.com/questions/2450954/how-to-randomize-shuffle-a-javascript-array) – Etheryte Jul 13 '15 at 19:40

2 Answers2

0

Your shuffle is a bit off, I looked at this which uses an array to shuffle.

// String concatenation
tp = rl + rn + rs;
tp=tp.split('');

// Shuffling part
for(var i = 0; i < tp.length; i++) {
   var rnd = Math.floor(Math.random() * tp.length);
   var temp = tp[i];
   tp[i] = tp[rnd];
   tp[rnd] = temp;
}
tp=tp.join("");
Community
  • 1
  • 1
depperm
  • 10,606
  • 4
  • 43
  • 67
  • Well that did the trick. Why is the 'split()' function necessary? – BrianRT Jul 13 '15 at 19:48
  • the split function makes an array from the string, so then the manipulation is using an array instead of a string, which isn't possible. – depperm Jul 13 '15 at 19:51
  • 1
    @daChi the `split` and `join` parts are necessary, because strings in javascript are immutable (as they are effectively values), while arrays are not. – doldt Jul 13 '15 at 19:52
  • Oh, so I was trying to shuffle a string when I thought I was shuffling an array. Got it. – BrianRT Jul 13 '15 at 19:54
  • There are ways to manipulate the string without turning it into an array. See my answer below. @doldt – coderfin Jul 13 '15 at 20:11
0

When you perform the assignments tp[i] = tp[rnd] and tp[rnd] = temp it is not changing the value of tp because strings are immutable. It is possible to manipulate a string and replace values using methods on String. Change those two assignments to the following:

for (var i = 0; i < tp.length; i++) {
    var rnd = Math.floor(Math.random() * tp.length);
    var temp = tp[i];
    tp = tp.replace(tp.substr(i, 1), tp[rnd]);
    tp = tp.replace(tp.substr(rnd, 1), temp);
}
coderfin
  • 1,708
  • 19
  • 24
  • It's worth noting that technically, you're not mutating the string in this case either: a new string is created, which is assigned to the `tp` variable instead of the old one. – doldt Jul 13 '15 at 21:14
  • This is interesting as well... which way would you say is better? Converting the string to an array, and then back to a string, or just doing string manipulations in and of itself? – BrianRT Jul 14 '15 at 15:15