JavaScript random loading message generator problem

Question

I don't know much about Javascript, and this function I wrote doesn't seem to be working properly. I don't know where it is going wrong, just that sometimes it doesn't work properly.

Any advice on how to improve it?

function GetRandomLoadingMessage() 
{
    var d = new Date();
    var weekday = d.getDay();
    var month = d.getMonth();

    var seed = weekday + "" + year;

    var choice = Math.round(Math.random(seed) * 2);

    if (choice == 1) {
        var lines = new Array(
        "Pay no attention to the man behind the curtain",
        "Follow the white rabbit",
        "The satellite is moving into position",
        "You're not in Kansas anymore",
        "Reticulating Splines",
        "The gods conemplate your fate",
        "It's not you, it's me",
        "So, do you come here often?",
        "Counting backwards from infinity",
        "Rolling for initiative");
    }

    if (choice == 2) {
        var lines = new Array(
        "E.T. phone home",
        "May the force be with you",
        "Here's looking at you, kid",
        "I'm gonna make him an offer he can't refuse",
        "Bond. James Bond.",
        "You're gonna need a bigger boat",
        "I'll be back",
        "Soylent Green is people!",
        "Open the pod bay doors, HAL",
        "Shaken, not stirred");

    }

    if (choice == 3) {
        var lines = new Array(
        "Parallel processors running perpendicular today",
        "Interference from lunar radiation",
        "Positron routers depleted",
        "Borg nanites infesting the server",
        "Astropneumatic oscillations inducing delays",
        "Increased sunspot activity",
        "Packets were eaten by the terminator",
        "Network packets travelling uphill",
        "Trojan horse ran out of hay",
        "Change in the earth's rotational speed");

    }

        return lines[Math.round(Math.random()*(lines.length-1))];

}

The idea, as you may have guessed, is to display messages from one particular "section" each day, at random... hence using a seed. Although I should change Year to Month or something.

Edit: Ok! Thanks for the information. I really need to figure out how to have a SEED though, like in C#. Is there a way? Each day I want it to choose a list and give responses from that list. By seeding from a combination of day/month, I could always use the same list for a given day.

Answer 1

First of all, you can seed the random with:

var n = new Date().getTime();

Then, use a switch statement instead of three separate ifs (or nested if/elses):

switch (choice) {
  case 1:
    ...
    break;
  case 2:
    ...
    break;
  case 3:
    ...
    break;
  default:
    ...
}

You might also consider just making one large array of choices if they need not be grouped like that and simply picking a random number between 0 and the length of the list - 1, then returning that index in the array:

var lines = [...];
function random_line() {
    var r = Math.random();
    var idx = Math.floor( r * (lines.length - 1) );
    return lines[idx];
}

Answer 2

You haven't declared the variable year, so the seed value will not quite look as you expect. WHen it's converted to be used in the random function it will most likely only use the weekday value. You probably meant to use the month variable instead:

var seed = weekday + "" + month;

Your random calculation is not correct. You will get half as many occurance of the first and the last items. You should use the floor method instead of round, and multiply with a value that is one greater:

var choice = Math.floor(Math.random(seed) * 3);

and

return lines[Math.floor(Math.random()*lines.length)];

Edit:
The value for choice of course varies from 0 to 2, not from 1 to 3.

Also, as Jon Benedicto pointed out in his answer, the random method doesn't take any parameter so the seed value will just be ignored. If you want a random value on a daily basis, you have to do your own pseudo random calculation. For example:

var now = new Date();
var choice = (now.getDay() * 251 + now.getDate()) % 3;

Answer 3

Use else if and replace if (choice == 3) with else to make sure lines is always assigned.

The declare lines outside the blocks (move var lines; up) and just assign lines in the code blocks. Otherwise, you'll get local variables that are removed when the code block in the if terminates.

Lastly, get Firefox and install Firebug and have a look into the error console for any further mistakes.

Answer 4

As per w3schools, Math.random() doesn't take any arguments. Does it?

Answer 5

There are a couple issues with that code.

1. Math.random() does not take any parameters, so you can remove that seeding code.

2. choice is going to be initialized with a number from 0 to 2, but you want it to be 1 to 3. Simple fix, add 1 to the value before assigning to choice:

   var choice = Math.round(Math.random() * 2) + 1;

3. Don't use Math.round(Math.random() * (n - 1)) to get the random range, use Math.floor(Math.random() * n). So your 2 random number generators become:

   var choice = Math.floor(Math.random() * 3) + 1;

   return lines[Math.floor(Math.random() * lines.length)];

Personally, I wouldn't bother with the 2 random numbers, I'd combine all the lines into one array, and use one random number.