0

I'm trying to create JS script that will generate random numbers between 2 values. But it doesn't work and I can't figure out why.

<form id="form" action="" method="get">
From: <input type="text" name="from" id="from"><br>
To: <input type="text" name="to" id="to"><br><br>
<button onclick="random_gen()">Generate</button>
</form>

    <script>
    function random_gen() {
    var a = document.getElementById("from").value;
    var b = document.getElementById("to").value;
    var c = Math.floor((Math.random() * b) + a);
    document.getElementById("final").innerHTML = c;
    return false;
    }
    </script>

<p id="final"></p>

PS I'm new at JS

9
  • Should be <button type=button ... Commented Jul 16, 2014 at 19:44
  • jsfiddle.net/7NDEn Commented Jul 16, 2014 at 19:46
  • @Pointy: why would button need a type? Commented Jul 16, 2014 at 19:49
  • @dandavis Technically, that button is submitting the form, so that could be the "problem" - the OP may see the page reload because the form is submitted (and therefore it looks like the .innerHTML = c; isn't happening. Then again, this may not be the problem the OP has at all Commented Jul 16, 2014 at 19:51
  • 1
    @Ian: that explains a lot ;) dan need more coffee... i knew it wasn't Pointy that was wrong... Commented Jul 16, 2014 at 19:53

2 Answers 2

3

Edited to reflect the comment from @blex, which is correct

The correct algorithm for random is:

function rand(a,b){
    return Math.floor(Math.random()*(b-a)+a);
}

When retrieving the values from an input, you must do parseInt:

var a = parseInt(document.getElementById("from").value);
var b = parseInt(document.getElementById("to").value);

Of course, you should check the values to make sure they are integers before you use them.

Sign up to request clarification or add additional context in comments.

2 Comments

and to add parseInt to a and b, and remove the <form> tags because the form gets sent instead of just executing the code. works with negative numbers: jsfiddle.net/7NDEn
You usually don't want to use parseInt with user input unless you want to let them put invalid characters at the end of a number. For example, parseInt("2342asdf") will result in 2342 (sometimes that isn't desired). I like to be explicit/strict and use Number("2342asdf") (or +"2342asdf") which would result in NaN - this converts to a number, unrelated to decimal points. In addition, you normally want to include the radix as the second parameter (10 for base 10) - parseInt("2342asdf", 10) - because some inputs may force the parser to use octal or hex, for example
0

Your element's values will be strings. You're going to wind up with a weird number, because you're generating a random number with Math.random() * b, which converts b to a number, and then concatenating the string contained in a to it.

Lets assume the following:

var a = '10';
var b = '20';

So, Math.random() * b works fine. It gives you a value like 15.12345. The problem is, you're concatenating the string "10" to the result, giving you a string, 15.1234510, which is passed to Math.floor, and converted back to the number 15.

You need to convert both of your inputs to numbers before passing them through your rand process. Use parseInt for this:

var a = parseInt(document.getElementById("from").value, 10);
var b = parseInt(document.getElementById("to").value, 10);

3 Comments

So, how should I do it?
See update. parseInt will turn both values into integers. If you want to use decimal values as your inputs, use parseFloat instead.
@meagar, the random must be multiplied by (b-a), otherwise his range is incorrect

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.