Help with indexOf()

J

Jim Davidson

I'm teaching myself javascript and need some help with this code. can
someone tell me what's wrong with it?
function getRandomNo()
{
while (true)
{
nImgNo = Math.round(Math.random() * 10);
var strImgNo = ""+nImgNo;

if (string_of_ImgNo.indexOf(strImgNo) = -1)
{
string_of_ImgNo = string_of_ImgNo+strImgNo;
break;
}
if (string_of_ImgNo.length > 9) break;
return nImgNo
}
}
 
L

Lee

Jim Davidson said:
I'm teaching myself javascript and need some help with this code. can
someone tell me what's wrong with it?
function getRandomNo()
{
while (true)
{
nImgNo = Math.round(Math.random() * 10);
var strImgNo = ""+nImgNo;

if (string_of_ImgNo.indexOf(strImgNo) = -1)
{
string_of_ImgNo = string_of_ImgNo+strImgNo;
break;
}
if (string_of_ImgNo.length > 9) break;
return nImgNo
}
}

1. You don't have a starting value for string_of_ImgNo.
2. You should never use Math.round() to generate random numbers [note 1].
Use Math.floor(), instead.
3. You don't need to convert numbers to strings. JavaScript doesn't care.
4. The comparison operator is "==", not "=".
5. You don't want to return nImgNo. You want string_of_ImgNo.
6. You don't want to "break" out of the loop until you're ready to
return, which means that you never want to use "break", at all.
Simply return when you're done, or better yet...
7. Change your while
loop condition to:
while(string_of_ImgNo.length<10)
and simply return string_of_ImgNo after the loop exits.
8. This isn't a very efficient way to generate a list of random
digits, anyway, but it will work if you fix the errors.


Note 1: Unless you're really sure you understand when it's ok.
In this case, using Math.round() will very often result in the
number "10" being added to your string, which would appear as
an extra 1 or 0 instead of some other digit, or possibly in
addition to all other digits, for a total of 11 digits.
 
J

Janwillem Borleffs

Jim Davidson said:
I'm teaching myself javascript and need some help with this code. can
someone tell me what's wrong with it? ....

if (string_of_ImgNo.indexOf(strImgNo) = -1)

'=' is an assignment, when you want to do a comparision use '==':
if (string_of_ImgNo.indexOf(strImgNo) == -1)

Also, string_of_ImgNo is undefined at this point, you should use strImgNo
instead when you really want to perform a string validation. However, this
isn't nessecary at all and you don't need to use the while loop either. When
the function is stripped down, you will end up with something like this:

function getRandomNo() {
return Math.round(Math.random() * 10);
}


JW
 
V

Vjekoslav Begovic

Jim Davidson said:
I'm teaching myself javascript and need some help with this code. can
someone tell me what's wrong with it?
function getRandomNo()
{
while (true)
{
nImgNo = Math.round(Math.random() * 10);
var strImgNo = ""+nImgNo;

if (string_of_ImgNo.indexOf(strImgNo) = -1)

And that is always true.

Equality operator in Javascript is ==.
 
L

Lasse Reichstein Nielsen

Janwillem Borleffs said:
When the function is stripped down, you will end up with something
like this:

function getRandomNo() {
return Math.round(Math.random() * 10);
}

And even then, you probably want either
return Math.floor(Math.random() * 10);
or
return Math.floor(Math.random() * 11);

The first gives a number between 0 and 9 with equal chance, the second
between 0 and 10 with equal chance. Using Math.round gives a number
between 0 and 10, with 0 and 10 having half the chance of the other
numbers.

/L
 
D

Dr John Stockton

JRS: In article <[email protected]>, seen
I'm teaching myself javascript and need some help with this code. can
someone tell me what's wrong with it?

It is possible to tell you some of what is wrong with it; for example,
string_of_ImgNo is undefined at the point of use, although that only
really matters if you intend to call the function getRandomNo.

But, as you have not said what you might want it to do, we cannot tell
you all that may be wrong with it, unless we guess the purpose.

My guess is that you want to return a ten-character string which is a
random permutation of '0123456789'; or that you want to return a digit
not previously used, which can be done by indexing into such a pre-
prepared permutation.

So you should read the newsgroup FAQ, searching for information on the
use of Random; the apparent problem is equivalent to dealing a deck of
ten cards 0..9. You will find a link to <http://www.merlyn.demon.co.uk/
js-randm.htm>.

Consider :

var RA = Deal(10), N=0, S=''

function Next() { return RA[N++] }

for (j=0; j<12; j++) S += Next()
document.writeln(RA, ' ',S)
 
J

Jim Davidson

Well I see I still have a lot to learn, thanks for the help...by the
way string_of_ImgNo is a Global variable defined earlier. Thanks
again. I'm sure I'll be asking more newbie questions.

Lee said:
Jim Davidson said:
I'm teaching myself javascript and need some help with this code. can
someone tell me what's wrong with it?
function getRandomNo()
{
while (true)
{
nImgNo = Math.round(Math.random() * 10);
var strImgNo = ""+nImgNo;

if (string_of_ImgNo.indexOf(strImgNo) = -1)
{
string_of_ImgNo = string_of_ImgNo+strImgNo;
break;
}
if (string_of_ImgNo.length > 9) break;
return nImgNo
}
}

1. You don't have a starting value for string_of_ImgNo.
2. You should never use Math.round() to generate random numbers [note 1].
Use Math.floor(), instead.
3. You don't need to convert numbers to strings. JavaScript doesn't care.
4. The comparison operator is "==", not "=".
5. You don't want to return nImgNo. You want string_of_ImgNo.
6. You don't want to "break" out of the loop until you're ready to
return, which means that you never want to use "break", at all.
Simply return when you're done, or better yet...
7. Change your while
loop condition to:
while(string_of_ImgNo.length<10)
and simply return string_of_ImgNo after the loop exits.
8. This isn't a very efficient way to generate a list of random
digits, anyway, but it will work if you fix the errors.


Note 1: Unless you're really sure you understand when it's ok.
In this case, using Math.round() will very often result in the
number "10" being added to your string, which would appear as
an extra 1 or 0 instead of some other digit, or possibly in
addition to all other digits, for a total of 11 digits.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
474,090
Messages
2,570,603
Members
47,223
Latest member
smithjens316

Latest Threads

Top