For non-Java heads, (int) Math.random() generates a random double from [0,1) which is then cast as an integer (which means that it is always truncated to 0).
They probably wanted something like (int) (Math.random()*2) which will give you 0 half the time or 1 half the time.
Although it is still a really inefficient way to generate 1-bit of entropy (but sometimes it just isn't worth the trouble to instantiate a Random object, so I guess I could look past that ...).
Aargh, no! That will seed the RNG on the basis of the current time. According to the docs, "Two Random objects created within the same millisecond will have the same sequence of random numbers" which really really isn't likely to be what you want.
To get sensible results, you need instead to keep a single Random object around and use it every time you need another bit. And yes, that's more trouble than just calling Math.random and tweaking the result.
Er, actually it looks as if exactly what this does depends on what version you're using. The Java Platform SE 6 docs say "This constructor sets the seed of the random number generator to a value very likely to be distinct from any other invocation of this constructor." Same for 5. But 1.4.2 has the text I quoted above.
In any case, creating a new Random object every time you want a random number is unlikely to give satisfactory results. There is certainly no guarantee that the implementation will try to make it do so.
Reread the docs! The API hasn't changed from 1.4 to 1.6. nextInt(x)'s is used to generate numbers from [0,x) Maybe in 1.6 they started using a better seeding source for randomness since they added System.nanoTime(). But, your original objection is a wrong interpretation of the API docs.
The point is that if every time you want a random number you construct a new Random object using the default constructor, then (1) if you're using version 1.4 or earlier then rapidly repeated "random" number generation will give you the same result over and over, and (2) if you're using version 1.5 or later then it's (quite rightly) almost entirely unspecified what you'll get and whether it will be "random enough" for any particular purpose and whether it will be terribly inefficient.
(I'm wondering whether perhaps you missed the fact that lucifer's proposed constructions involve making a new Random object each time you want some randomness.)
Still, if the point of using the Random object is being efficient, instantiating an object for just one nextBoolean() call is likely to be more inefficient than just calling Math.random().
But, gjm11, seeding has nothing to do with topic we were discussing which was trying to random generate a number between [0,x). So your objection still exists even in the bugs because all examples were using the no-arg constructor (Math.random() uses Random under the covers with a no-arg constructor). This is a very confusing reply you wrote because you're changing the topic!
According to 1.4.2 docs, and this hasn't changed. If you want a distribution of numbers, say int, from 0-x you should do the following:
new Random().nextInt( x )
Don't do what the bug pointed out (this is wrong and ugly quite frankly!):
(int)new Random() * x
If you follow the first example then you avoid the bug this post was about.
If you find objection similar to gjm11, aka same seed problem, then I'd say you need to graduate out of Random to SecureRandom. But, in case you just can't let go here's how you fix that:
new Random( seed ).nextInt( x );
But, if you feel like now is the time to take off the training wheels here's SecureRandom:
SecureRandom.getInstance("SHA1PRNG").nextInt(x);
That source has more randomness which is suitable for things like cryptography, and places where gjm11 statement, aka same seed problem, matters.
Math.random calls new Random() only once, the first time you invoke it. After that, it continues to use the same Random object.
If you say new Random().nextInt(x) every time you want a new random number, the documentation (very reasonably) gives you nothing even slightly resembling any sort of guarantee about what sort of distribution the results will have.
The point isn't that there's something wrong with using the no-argument constructor for Random. The point is that there's something wrong with making a new Random object every time you want a new random number. It's likely to be inefficient; there's no guarantee that the code makes any effort to generate good random numbers when you do it; and in versions 1.4 and earlier there was in fact a guarantee that it doesn't.
I was "changing the topic" because I was replying to someone who made a specific suggestion which happens to be a bad suggestion. To be clear: I wasn't objecting to lucifer's suggestion of using nextInt and nextBoolean. Those are good suggestions. The point is that in order to get the same little bit of convenience that you get from calling Math.random -- namely, not needing to set up a Random object and make it available to everything in your code that needs random numbers -- lucifer's suggestion does something that turns out to be bad, namely constructing a new Random object every time.
SecureRandom is rather a red herring. In any case, calling SecureRandom.getInstance every time you want a random number is liable to be extremely inefficient. Like, multiple seconds per call [EDITED: following three words added] on some implementations. See, e.g., http://marc.info/?l=wss4j-dev&m=119122666202674&w=2 .
"The point isn't that there's something wrong with using the no-argument constructor for Random."
This point was never mentioned in your original post. Instead ran off and started talking about seeding issues which was completely not related to the original topic.
If your problem was creating a new instance every time you get a random number it seems extremely trivial to remedy that (using Random or SecureRandom).
Random rand = new Random();
for( int i = 0; i < 10000; i++ ) {
numbers[i] = rand.nextInt( x );
}
That seems very obvious if efficiency is your issue here!
"To be clear: I wasn't objecting to lucifer's suggestion of using nextInt and nextBoolean."
This was NOT even close to clear from your original post. It read like your were saying don't use this method because of issues with seeding.
My problem was you were handing out worse advice without any justification because your original post could be misinterpreted as I did. It sounded like you preferred people call Math.random() which was leading them back towards the user unfriendly version Math.random() which is the source of so many bugs when there are better options.
Not too much trouble, but horribly wrong. You are generating a new Random instance with possibly expensive seed generation for every random number, and, even worse, you are completely at the mercy of the seed generation used by your JVM.
Sun's is relatively good:
public Random() { this(++seedUniquifier + System.nanoTime()); }
But GNU classpath contains:
public Random()
{
this(System.currentTimeMillis());
}
Which will give you the same predictible "random" number for every call within a millisecond.
The point is that while it's not too much trouble to do as you did, it is often too much trouble to do it properly. Giving the OP some credit for being a reasonable programmer, your reply is less useful than it might at first appear.
Sure, it isn't too much trouble to instantiate a Random object, but to do it properly can be a right pain, and sometimes just isn't worth it.
I don't think that the first one is a bug. First, no programmer with any experience would write (variable == true) instead of (variable). Second, there are a number of lines commented out in that area so it's likely that that particular code was intended as a "temporary hack". I think the intention of the author was definitely assignment in that case, although it is expressed in a hackish, unreadable way. Seeing the previous revisions of the same file would be helpful.
EDIT/TLDR: It looks like it either is intentional, or written by very unexperienced developer.
mAutoCommit looks like it can be toggled to true/false to enable automatic updating of some configuration inside the setValue(Object) method. I'm 99.9999 percent sure that the author of the code did not intend to toggle the mAutoCommit option to 'true' each time a configuration value is set.
I will agree, however, that this is almost certainly written by an inexperienced developer. It's not a stretch to think that the inexperienced developer who might test a boolean with x == true might also forget an equals sign.
I write "variable == true" occasionally. I must not have any experience though; I developed the habit of being very specific after dealing with quirky languages like, say, PHP.
If you want to know if the first example in that list is a bug or not, all you have to do is click on it and read the linked code. (Hint: it's a bug.)
I love this bug! It is deeply ingrained in my head that == is the right thing to use, but I still use the convention of placing the test value first (i.e. 4 == numItems) to be sure I don't ever do this.
class Test {
public Test() {}
public static void main(String[] argv) {
int i;
for (i = 0; i < 100; i += 1) {
System.out.println("RND=" + ((int) Math.random() * 100000));
}
}
}
They probably wanted something like (int) (Math.random()*2) which will give you 0 half the time or 1 half the time.
Although it is still a really inefficient way to generate 1-bit of entropy (but sometimes it just isn't worth the trouble to instantiate a Random object, so I guess I could look past that ...).