patternjavascriptMinor
Generating random numbers and decks of cards
Viewed 0 times
randomcardsnumbersgeneratinganddecks
Problem
So I have some code here:
First a simple random # generator and a array-choosing function:
and second a Card shuffler:
Is there an easy way to make this faster or at least declare the arrays faster instead of just doing a=[],b=[]...
Additionally, is there a way to get a better RNG with a longer period in base JS (no libraries, as this was built on base JS)
First a simple random # generator and a array-choosing function:
function Rand(min,max)
{
return parseFloat(Math.floor(Math.random()* max-min+1)))+parseFloat(min);
}
function Choose(arr)
{
//Returns an element from an array at random.
return arr[Math.floor(Math.random()*arr.length)];
}and second a Card shuffler:
function CardDeck() {
var Cd=["Ace","2","3","4","5","6","7","8","9","10","Jack","Queen","King"];
var H=[];
var S=[];
var D=[];
var C=[];
var Result=[];
var Dk=document.getElementById("Deck Count").value;
for(i=0;i<Cd.length;i++) {
S[i]=Cd[i]+" of Spades";
H[i]=Cd[i]+" of Hearts";
C[i]=Cd[i]+" of Clubs";
D[i]=Cd[i]+" of Diamonds";}
if (Dk=="4") {P=S.concat(C,D,H);}
else if (Dk=="5") {
var St=[];
for(i=0;i<Cd.length;i++) {St[i]=Cd[i]+" of Stars";}
P=S.concat(C,D,H,St);}
else if (Dk=="6") {
var Rk=[];
var Wh=[];
for(i=0;i<Cd.length;i++) {
Rk[i]=Cd[i]+" of Rackets";
Wh[i]=Cd[i]+" of Wheels";}
P=S.concat(C,D,H,Rk,Wh);}
for(i=0;i<Dk*Cd.length;i++) {
var Q=Choose(P);
R=P.indexOf(Q);
Result[i]=(i+1)+": "+Q;
P=P.slice(0,R).concat(P.slice(R+1));}
document.getElementById("Cards").innerHTML=Result.join("\n");}Is there an easy way to make this faster or at least declare the arrays faster instead of just doing a=[],b=[]...
Additionally, is there a way to get a better RNG with a longer period in base JS (no libraries, as this was built on base JS)
Solution
First off, your code is borderline unreadable. Use proper indentation. Add whitespace (horizontal and vertical). Follow conventional guidelines (lowercase camelCase names for most things). And give your variables actual names - bytes are cheap.
If you want really compact code, use a minifier - after writing the code in readable, sane manner. This looks like a text dump of a fiendish Excel spreadsheet or something.
For the few things that I could read, and which stand out:
-
You
And it's broken (which makes it technically off-topic for CodeReview, by the way).
Firstly, it doesn't even run because you've got one close-parenthesis too many, so there's a syntax error.
Secondly, you're not paying attention to operator precedence: Multiplication precedes addition and subtract. So your code is saying
Just say:
-
As for
-
When appending to and array, use
As for "better RNG", that's a StackOverflow question if anything. JavaScript has
If you want really compact code, use a minifier - after writing the code in readable, sane manner. This looks like a text dump of a fiendish Excel spreadsheet or something.
For the few things that I could read, and which stand out:
-
You
Rand [sic] function is doing a bunch of unnecessary stuff. You parseFloat something that's guaranteed to be an integer, because you called Math.floor() on it. You parseFloat on min when adding it at the end, but you don't do it when you subtract it from max.And it's broken (which makes it technically off-topic for CodeReview, by the way).
Firstly, it doesn't even run because you've got one close-parenthesis too many, so there's a syntax error.
Secondly, you're not paying attention to operator precedence: Multiplication precedes addition and subtract. So your code is saying
Math.random() * max then subtracting min and finally adding 1. Hence, if you call Rand(9, 10), you're liable to get any number from 1-10, rather than a number from 9-10.Just say:
function rand(min, max) {
var span = max - min;
return Math.round(Math.random() * span) + min;
}-
As for
Choose [sic], you can use the bitwise-floor trick:function choose(arr) {
return arr[Math.random() * arr.length | 0];
}-
When appending to and array, use
push. Don't just assign to an index. E.g.spades.push(values[i] + " of Spades");
hearts.push(values[i] + " of Hearts");
clubs.push(values[i] + " of Clubs");
diamonds.push(values[i] + " of Diamonds");As for "better RNG", that's a StackOverflow question if anything. JavaScript has
Math.random() and that's it.Code Snippets
function rand(min, max) {
var span = max - min;
return Math.round(Math.random() * span) + min;
}function choose(arr) {
return arr[Math.random() * arr.length | 0];
}spades.push(values[i] + " of Spades");
hearts.push(values[i] + " of Hearts");
clubs.push(values[i] + " of Clubs");
diamonds.push(values[i] + " of Diamonds");Context
StackExchange Code Review Q#136602, answer score: 5
Revisions (0)
No revisions yet.