Another way of using index for the random password?

Unity Courses Talk
, ,
#1

Pasting the random number generator to get the index of a random password seems a bit…ugly? I get what you’re saying in the course that setting the index at the top causes problems, but what if you only initialise the variable? This is what I have…

void StartGame ()
{
    int index;
    // Other code omitted for simplicity's sake...

    switch (level)
    {
        case 1:
            index = Random.Range(0, level1Passwords.Length);
            password = level1Passwords[index];
            break;
        case 2:
            index = Random.Range(0, level2Passwords.Length);
            password = level2Passwords[index];
       default:
           Debug.LogError("Invalid level number");
    }
}

For me, it’s much more readable to use the index variable, but I’m wondering if there are any issues with doing it like this? I don’t seem to have any but as said in the course video, it’s a potentially nasty bug as it’s difficult to come across, but the potential is there.

#2

There’s usually a million different ways to code a solution, it’s really up to you, there shouldn’t be a syntax problems with the way you do it, however you’re just adding extra lines when you can put it all together, and that’s fine, i think what Ben means is it is just generally good practice to keep it short and sweet.

I am in the same boat as you, i prefer putting all of my variables in game state as opposed to the method though, just because i often lose track of where i put my variables in code.

#3

you can also declare the index variable in case 1 and use it again in case 2 like so:

            case 1:
                int index = Random.Range(0, level1Passwords.Length);
                password = level1Passwords[index];
                break;
            case 2:
                index = Random.Range(0, level2Passwords.Length);
                password = level2Passwords[index];
                break;
#4

Ben’s solution not to store the variable is probably the best but at the cost of readability imho. Personally I would do an extract to a method that takes the array as a parameter and returns the randomly selected password but we haven’t seen non void methods at this point.

#5

Initially I wrote mine the same way as Obiwan_Kenobi which worked OK but seeing the course end code, it did seem better, less code and no definition of index needed. Less to go wrong.

1 Like
#6

This is mostly a matter of style vs. readability. The actual storage of index is unneccesary, since programmatically, it’s perfectly acceptable to pass the Random.Range(0,level1Passwords.length). You’re not using index again, you’re only using it to index into the passwords array, so there’s not a programmatic justification for storing (even temporarily) index.
That being said, perhaps you’re going to do other things with that index… (maybe the password and a crossword puzzle type hint, for example, each in its own array). Then you’d want to store the index.

There’s nothing wrong with storing the index, though, if it helps you to read the code better at a later date.

#7

Yes, this is exactly how I wrote my code, I don’t know why Ben didn’t come up with this solution in the first place. It makes the code simple, you are not creating extra un-needed variables and it is certainly expandable.

Edit: guys, he actually does this solution near the end of the video.

#8

I came here to see if anyone else used this solution as well. This seems simpler to me, at least for a novice.
I realise what Ben showed is probably easier to read overall since you generate and store the value on a single line, but for me the overall logic is simpler this way round.

Pleased to see I’m not alone.

Privacy & Terms