# logic clean up in utils.js

RESOLVED FIXED

## Status

Other Applications
ChatZilla
--
minor
RESOLVED FIXED
16 years ago
13 years ago

Trunk
---

## Attachments

### (1 attachment, 2 obsolete attachments)

 1.01 KB, patch Robert Ginda : review+ Details | Diff | Splinter Review
(Reporter)

### Description

16 years ago
```Noticed the following issues in utils.js, and think I have alternatives which
will save some cycles and possibly improve readability.

1) Unneeded use of parseInt() to find integer part of an expression.
Fix: replace each parseInt() with Math.floor() globally.

2) Suggest the following replacements:

function getRandomElement (ary)
{
return ary[Math.floor (Math.random() * ary.length)];
}

function roundTo (num, prec)
{
var tp = Math.pow (10, prec);

return Math.floor (Math.round(num * tp)) / tp;
}

3) Unsure how randomRange() is supposed to work.  It really depends on whether
we are assuming any/all integer inputs, and if the output should be integer.  My
guess is that what is wanted is a random integer between integers min and max,
inclusive.

This would be:

function randomRange (min, max)
{
if (typeof min == "undefined") min = 0;
if (typeof max == "undefined") max = 1;

return min + Math.floor(Math.random() * (max - min + 1));
}```

### Comment 1

16 years ago
```Created attachment 67223 [details] [diff] [review]

### Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: patch
(Assignee)

### Comment 2

16 years ago
``` function getRandomElement (ary)
{
-    var i = parseInt (Math.random() * ary.length)
-
if (i == ary.length) i = 0;

-    return ary[i];
+    return ary[Math.floor(Math.random() * ary.length)];

}

There is no element ary[ary.length], which is what that wacky test was looking
out for.  Perhaps return ary[Math.floor(Math.random() * (ary.length - 1))];

The other two hunks have extra parens around the first Math.round().

fis that and r=rginda.```
(Reporter)

### Comment 3

16 years ago
```Math.random() returns numbers < 1, so the array would never get indexed at
ary.length.  To illustrate, assume ary.length == 5, and run this as many times
as you want:

You'll never generate a 5.  Changing to length - 1 would mean that the last
element would never be returned.  Again, with ary.length == 5, we would have
Math.random() * 4, which would only generate 0, 1, 2, and 3, but never a 4, even
though that's a valid index.```

### Comment 4

16 years ago
```ok, from the spec: "[random()] Returns a number value with positive sign,
greater than or equal to 0 but less than 1"```

### Comment 5

16 years ago
```Created attachment 69007 [details] [diff] [review]
new patch, missed randomRange```
Attachment #67223 - Attachment is obsolete: true

### Comment 6

16 years ago
```Created attachment 69008 [details] [diff] [review]
another try

a little too quick on the submitting```
Attachment #69007 - Attachment is obsolete: true
(Assignee)

### Comment 7

16 years ago
```Comment on attachment 69008 [details] [diff] [review]
another try

r=rginda```
Attachment #69008 - Flags: review+

### Comment 8

16 years ago
`checked in`
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

### Updated

13 years ago
Product: Core → Other Applications
You need to log in before you can comment on or make changes to this bug.