can this code be improved

P

Print Guy

Hi all.

To most of you, what I have been working on is probably trivial and
easy, but it's taken me the good part of a week working 2 or 3 hrs a
day to finally figure out how to do what I needed to do.
Here in Canada, we have a lottery called 6-49. You pick 6 numbers
between 1 and 49. Those numbers are put on a ticket and there is a
draw twice a week.

I wanted to come up with a statistically solid way to pick my numbers
so I figured that if I were to pick 6 numbers 1,000,000 times and count
the number of times each number is selected, the top six would be good
numbers to bet on during the lottery.

With that being said, I thought I could write a Java program to do the
job. It proved to be a bit more difficult than I thought it would be;
the hardest part being ensuring that each of the 6 numbers were unique.

Here is my code. What I am hoping for is some constructive criticism
which could help me to make the code more efficient.

(Normally I would not comment my code so much, but I wanted you to see
what I was trying to accomplish)

package lotto;

import java.util.Arrays;
import java.util.Date;

import java.util.Random;

public class Sample {
public static void main(String args[]) {
Date today=new Date();
System.out.println("Starting:" + today);
// the original array of 6 numbers picked
int [] list = new int[6];
// the new array containing unique numbers only
int[] newList = new int[6];
int number = 0;
// this array is used to count the number of occurances (1-49)
int [] BigList = new int[50];
// initialize BigList
for ( int x=0;x<=BigList.length-1;x++)
BigList[x]=0;
// do this whole thing a million times
for (int y=1;y<=1000000;y++) {
// initialize newList
for (int t=0;t<=newList.length-1;t++)
newList[t]=0;
// put 6 random numbers into an array
for ( int z=0;z<6;z++) {
Random rand = new Random() ;
number=rand.nextInt(49) + 1;
list[z]=number;
}
int count=0;
// what this loop does is check each member of "list" to see
// if it's a duplicate. Copy the current element to the newList
// if it's not a duplicate.
boolean dupLocated=false;
for ( int b=0;b<=list.length-1;b++)
{
int chkNum=list;
int temp = b;
boolean found = false;
for ( int a=0;a<=newList.length-1;a++)
{
if ( newList[a] == chkNum)
{
found=true;
dupLocated=true;
}
}
if ( ! found )
newList[temp]=chkNum;
}
// now that we have at least one duplicate, we need to replace
// the zero space holder with a new random number. So
// to do that, we select a random number and then check to see
// if it is the array. If it isn't then stick in in the list
otherwise, keep
// selecting and checking until we can put it in (usually takes one or
two
// iterations
if ( dupLocated )
{
for (int x=0;x<=newList.length-1;x++)
if ( newList[x] == 0 )
{
boolean t=false;
while ( !t )
{
Random rand = new Random() ;
int number1=rand.nextInt(49) + 1;
int tt = 0;
for (int r=0;r<=newList.length-1;r++)
if ( newList[r] != 0 )
if ( newList[r] == number1 )
tt++;
if ( tt == 0 )
{
newList[x]=number1;
t=true;
}
}
}
}
// so now we have a list of 6 unique numbers. and we need to increment
// the value in BigList that corresponds to each of the six numbers.
for (int j=0;j<=list.length-1;j++)
{
int idx=list[j];
BigList[idx]++;
}

}
// now find the top six and create a new array called topSix
int [] topSix = new int[6];
for (int count=0;count<=topSix.length-1;count++)
{
int lIndex=0;
int largest=BigList[0];
// compare each element of BigList with each "other" element
// of BigList (I created this algorithm on my own, or maybe I rememered
// it from somewhere.
for (int x=0;x<=BigList.length-1;x++)
{
for ( int y=x+1;y<=BigList.length-2;y++)
if ( BigList[y] > largest )
{
largest=BigList[y];
BigList[y]=0;
lIndex=y;
topSix[count]=lIndex;
}

}
}
// now print out the list of topSix
for (int i=0;i<=topSix.length-1;i++)
System.out.println("Rank " + (i+1) + " number is " + topSix);
Date today2=new Date();
System.out.println("Ending:" + today2);
}
}

------------------------------------------------------------------------------
Sample output

Starting:Wed Aug 16 20:51:49 ADT 2006
Rank 1 number is 43
Rank 2 number is 48
Rank 3 number is 29
Rank 4 number is 30
Rank 5 number is 35
Rank 6 number is 46
Ending:Wed Aug 16 20:52:04 ADT 2006

15 seconds isn't too bad... but maybe it could be faster?
 
A

Andrew Thompson

Print said:
Hi all.

To most of you, what I have been working on is probably trivial and
easy, but it's taken me the good part of a week working 2 or 3 hrs a
day to finally figure out how to do what I needed to do.
Here in Canada, we have a lottery called 6-49. You pick 6 numbers
between 1 and 49. Those numbers are put on a ticket and there is a
draw twice a week.

I wanted to come up with a statistically solid way to pick my numbers
so I figured that if I were to pick 6 numbers 1,000,000 times and count
the number of times each number is selected, the top six would be good
numbers to bet on during the lottery.

I am no statistician, so this is more a question than a strategy,
but woul it not be just as valid to have a Vector of
'number of values in lottery' and simply remove any number
that comes up?

E.G. start with a vector of 49 elements, if '35' is first
chosen, remove that element and choose randomly
from the the remaining 48 elements.
public class Sample {
public static void main(String args[]) {
Date today=new Date();

And please change 'tab's to a couple of spaces before posting,
that line is this wide...
Date today=new Date();
...in my 'news client'.

Andrew T.
 
P

Print Guy

Andrew said:
Print said:
Hi all.

To most of you, what I have been working on is probably trivial and
easy, but it's taken me the good part of a week working 2 or 3 hrs a
day to finally figure out how to do what I needed to do.
Here in Canada, we have a lottery called 6-49. You pick 6 numbers
between 1 and 49. Those numbers are put on a ticket and there is a
draw twice a week.

I wanted to come up with a statistically solid way to pick my numbers
so I figured that if I were to pick 6 numbers 1,000,000 times and count
the number of times each number is selected, the top six would be good
numbers to bet on during the lottery.

I am no statistician, so this is more a question than a strategy,
but woul it not be just as valid to have a Vector of
'number of values in lottery' and simply remove any number
that comes up?

E.G. start with a vector of 49 elements, if '35' is first
chosen, remove that element and choose randomly
from the the remaining 48 elements.
public class Sample {
public static void main(String args[]) {
Date today=new Date();

And please change 'tab's to a couple of spaces before posting,
that line is this wide...
Date today=new Date();
..in my 'news client'.

Andrew T.

Sorry man... eclipse did the "indentation" all I did was copy and
paste into google.groups

Yeah.. the vector idea sounds good except that I haven't gotten that
far in my self-education process... I wanted to see if I could
accomplish my goal with the tools and knowledge I currently have....
but... I like your idea and as soon as I get that far in my learning
process I will try to write something..
 
S

Stefan Ram

Print Guy said:
Here in Canada, we have a lottery called 6-49.

No, this lottery in fact is located here in Germany, and it's
called "6 aus 49".
I wanted to come up with a statistically solid way to pick my
numbers so I figured that if I were to pick 6 numbers 1,000,000
times and count the number of times each number is selected,
the top six would be good numbers to bet on during the lottery.

Actually the numbers are best, which are most rarely chosen
by other players, because then the rates will be higher.
Here is my code. What I am hoping for is some constructive criticism
which could help me to make the code more efficient.

Destructive criticism is much more fun!
Rank 1 number is 43

class NumericMapUtils
{ public static <D> void addTo
( final java.util.Map<D,java.lang.Integer> map, final D d, final int i )
{ map.put( d, i +( map.containsKey( d )? map.get( d ): 0 )); }}

public class Main
{ static final java.util.Random rand = new java.util.Random();
public static void main( java.lang.String[] args )
{ final java.util.Map<java.lang.Integer,java.lang.Integer> map
= new java.util.HashMap<java.lang.Integer,java.lang.Integer>( 50 );
for( int i = 0; i < 1000; ++i )
NumericMapUtils.<java.lang.Integer>addTo( map, rand.nextInt( 49 ), 1 );
final java.util.SortedMap<java.lang.Integer,java.lang.Integer> sort
= new java.util.TreeMap<java.lang.Integer,java.lang.Integer>();
for( final java.lang.Integer i : map.keySet() )sort.put( -map.get( i ), i );
int c = 0; for( final int i : sort.keySet() )
{ java.lang.System.out.println
( "Rank " +( c + 1 )+ " number is " + sort.get( i ));
if( ++c >= 6 )break; }}}

Rank 1 number is 21
Rank 2 number is 14
Rank 3 number is 34
Rank 4 number is 15
Rank 5 number is 47
Rank 6 number is 20

However, there is a small chance that »nextInt« will return
the same number for 1000 times, so that the program would only
output one number; but I tried to implement your general
description.
 
S

Stefan Ram

sort.put( -map.get( i ), i );

This attempt to "invert" a map still has a bug: If two keys of
the original map have the same value, one of them will be
lost. The following solution uses a small "bias" added to
each value, to make it different from every other value.
(It might fail under some conditions.)

class NumericMapUtils
{ public static <D> void addTo
( final java.util.Map<D,java.lang.Integer> map, final D d, final int i )
{ map.put( d, i +( map.containsKey( d )? map.get( d ): 0 )); }}

public class Main
{ static final java.util.Random rand = new java.util.Random();
public static void main( java.lang.String[] args )
{ final java.util.Map<java.lang.Integer,java.lang.Integer> map
= new java.util.HashMap<java.lang.Integer,java.lang.Integer>( 50 );
final int k = 1000; for( int i = 0; i < k; ++i )
NumericMapUtils.<java.lang.Integer>addTo( map, rand.nextInt( 49 ), 1 );
final java.util.SortedMap<java.lang.Double,java.lang.Integer> sort
= new java.util.TreeMap<java.lang.Double,java.lang.Integer>();
int j = 0; for( final java.lang.Integer i : map.keySet() )
sort.put( -map.get( i )+ j++ * .8 / k, i );
int c = 0; for( final java.lang.Double d : sort.keySet() )
{ java.lang.System.out.println
( "Rank " +( c + 1 )+ " number is " + sort.get( d ));
if( ++c >= 6 )break; }}}

A more clean solution might replace each key of the map
"sort" by a ComparablePair of both the key and its value.

I even have implemented such a beast

http://www.purl.org/stefan_ram/html/ram.jar/de/dclj/ram/type/pair/ComparablePair.html
 
L

Luke Webber

Print said:
Hi all.

To most of you, what I have been working on is probably trivial and
easy, but it's taken me the good part of a week working 2 or 3 hrs a
day to finally figure out how to do what I needed to do.
Here in Canada, we have a lottery called 6-49. You pick 6 numbers
between 1 and 49. Those numbers are put on a ticket and there is a
draw twice a week.

I wanted to come up with a statistically solid way to pick my numbers
so I figured that if I were to pick 6 numbers 1,000,000 times and count
the number of times each number is selected, the top six would be good
numbers to bet on during the lottery.

With that being said, I thought I could write a Java program to do the
job. It proved to be a bit more difficult than I thought it would be;
the hardest part being ensuring that each of the 6 numbers were unique.

Here is my code. What I am hoping for is some constructive criticism
which could help me to make the code more efficient.

(Normally I would not comment my code so much, but I wanted you to see
what I was trying to accomplish)
[snip]

If you're asking this just as a Java question, then there are certainly
things you can do to improve on it. But I have to point out that you're
not modelling the real world here. You'll mostly be testing for bias in
the Java random number generator, when you should really be testing for
bias in whatever process/equipment the lottery agency is using.

If they keep historical records for downloading (as our Australian
agency does), you'd be better served to download that and massage it.

Cheers,
Luke
 
P

Patricia Shanahan

Print said:
Hi all.

To most of you, what I have been working on is probably trivial and
easy, but it's taken me the good part of a week working 2 or 3 hrs a
day to finally figure out how to do what I needed to do.
Here in Canada, we have a lottery called 6-49. You pick 6 numbers
between 1 and 49. Those numbers are put on a ticket and there is a
draw twice a week.

I wanted to come up with a statistically solid way to pick my numbers
so I figured that if I were to pick 6 numbers 1,000,000 times and count
the number of times each number is selected, the top six would be good
numbers to bet on during the lottery.

You are assuming that any bias in the Java Random class also applies to
the lottery's random number generation method? I suspect it is using
genuinely random numbers, not just pseudo-random, and monitors them for
bias.
With that being said, I thought I could write a Java program to do the
job. It proved to be a bit more difficult than I thought it would be;
the hardest part being ensuring that each of the 6 numbers were unique.

Here is an alternative method for picking the 6 numbers.

There is a well-known linear time algorithm for shuffling an array, the
Fisher-Yates shuffle. On iteration i, the element with index i is
swapped with a randomly selected element between i and the end of the array.

There are two important loop invariants:

1. At the end of every iteration, the array contains a permutation
of its original contents.

2. After N iterations, the first N elements of the array are randomly
selected with equal probability.

In a normal shuffle, the number of iterations is equal to the size of
the array minus one, and all elements have been shuffled. (The last
iteration is not needed because it makes a random choice from one
element, and swaps that element with itself).

Why not run Fisher-Yates for 6 iterations, and then pick up the
first 6 elements of the array?

Patricia



/**
* Partially shuffle the elements of an int
* array
*
* On completion, data contains a permutation of
* its original contents, with the elements in
* the first count spaces selected randomly.
* There is no assurance of randomness for
* elements data[count] and later, and many of
* those elements may remain in their original
* positions.
*
* @param data
* The array
* @param count
* The number of elements to shuffle.
* If count is negative or greater than
* data.length, all elements are
* shuffled.
* @param rand
* partialShuffle gets its random
* numbers from this generator.
*/
public static void partialShuffle(int[] data, int count,
Random rand) {
if (count > data.length || count < 0) {
count = data.length;
}
for (int i = 0; i < count; i++) {
int pickIndex = rand.nextInt(data.length - i) + i;
int temp = data[pickIndex];
data[pickIndex] = data;
data = temp;
}
}
 
B

bugbear

Print said:
Hi all.

To most of you, what I have been working on is probably trivial and
easy, but it's taken me the good part of a week working 2 or 3 hrs a
day to finally figure out how to do what I needed to do.
Here in Canada, we have a lottery called 6-49. You pick 6 numbers
between 1 and 49. Those numbers are put on a ticket and there is a
draw twice a week.

I wanted to come up with a statistically solid way to pick my numbers

This requires analysis, not code.

BugBear
 
A

AndrewMcDonagh

Print said:
Hi all.

To most of you, what I have been working on is probably trivial and
easy, but it's taken me the good part of a week working 2 or 3 hrs a
day to finally figure out how to do what I needed to do.
Here in Canada, we have a lottery called 6-49. You pick 6 numbers
between 1 and 49. Those numbers are put on a ticket and there is a
draw twice a week.

I wanted to come up with a statistically solid way to pick my numbers
so I figured that if I were to pick 6 numbers 1,000,000 times and count
the number of times each number is selected, the top six would be good
numbers to bet on during the lottery.

With that being said, I thought I could write a Java program to do the
job. It proved to be a bit more difficult than I thought it would be;
the hardest part being ensuring that each of the 6 numbers were unique.

Here is my code. What I am hoping for is some constructive criticism
which could help me to make the code more efficient.

snipped

ok, to be brutal - you have created a Procedural solution not an Object
Oriented solution. Given that Java is aimed at being an OO language
(lets not get into an argument about what that is nor Java's flaws) it
would be good to see the solution using the tools and techniques from an
OO perspective.

That said, even if we stick with a procedural solution your current one
needs a lot of work to improve its readability (never mind logic changes
to remove unneeded loops, conditions or state)

HTH

Andrew...........


A simple starting point would be to turn comments into executable comments.

For example,

// the original array of 6 numbers picked
int [] list = new int[6];
// the new array containing unique numbers only
int[] newList = new int[6];
// this array is used to count the number of occurances (1-49)
int [] BigList = new int[50];

becomes something like...

int [] originalPickedNumbers = new int[6];
int [] uniqueNumbers = new int[6];
int [] numberOfOccurancesCount = new int[50];



Another example would be for each of those comments blocks to become
method names and move the code they refer to, into these new methods.

for example....

public class Sample {

public static void main(String args[]) {
Date today = new Date();
System.out.println("Starting:" + today);

//the original array of 6 numbers picked
int[] list = new int[6];
//the new array containing unique numbers only
int[] newList = new int[6];
int number = 0;
//this array is used to count the number of occurances (1-49)
int[] BigList = new int[50];

initializeBigList(BigList);
//do this whole thing a million times
for (int y = 1; y <= 1000000; y++) {
initializeNewList(newList);
put6RandomNumbersIntoAnArray(list);
boolean dupLocated = copyAllUniqueNumbersIntoNewList(list, newList);

if (dupLocated) {
replaceAnyZeroSpaceHolderWithRandomNumber(newList);
}

incrementBigListValuesThatMatchEachOfTheSixNumbers(list, BigList);
}

int[] topSix = createNewListContainingOnlyTopSix(BigList);
printListOfTopSix(topSix);
}




private static void printListOfTopSix(int[] topSix) {
//now print out the list of topSix
for (int i = 0; i <= topSix.length - 1; i++)
System.out.println("Rank " + (i + 1) + " number is " + topSix);
Date today2 = new Date();
System.out.println("Ending:" + today2);
}

private static int[] createNewListContainingOnlyTopSix(int[] BigList) {
//now find the top six and create a new array called topSix
int[] topSix = new int[6];
for (int count = 0; count <= topSix.length - 1; count++) {
int lIndex = 0;
int largest = BigList[0];
//compare each element of BigList with each "other" element
//of BigList (I created this algorithm on my own, or maybe I
rememered
//it from somewhere.
for (int x = 0; x <= BigList.length - 1; x++) {
for (int y = x + 1; y <= BigList.length - 2; y++)
if (BigList[y] > largest) {
largest = BigList[y];
BigList[y] = 0;
lIndex = y;
topSix[count] = lIndex;
}

}
}
return topSix;
}

private static void
incrementBigListValuesThatMatchEachOfTheSixNumbers(int[] list, int[]
BigList) {
//so now we have a list of 6 unique numbers. and we need to increment
//the value in BigList that corresponds to each of the six numbers.
for (int j = 0; j <= list.length - 1; j++) {
int idx = list[j];
BigList[idx]++;
}
}

private static void replaceAnyZeroSpaceHolderWithRandomNumber(int[]
newList) {
//now that we have at least one duplicate, we need to replace
//the zero space holder with a new random number. So
//to do that, we select a random number and then check to see
//if it is the array. If it isn't then stick in in the list
//otherwise, keep
//selecting and checking until we can put it in (usually takes one
ortwo
//iterations
for (int x = 0; x <= newList.length - 1; x++)
if (newList[x] == 0) {
boolean t = false;
while (!t) {
Random rand = new Random();
int number1 = rand.nextInt(49) + 1;
int tt = 0;
for (int r = 0; r <= newList.length - 1; r++)
if (newList[r] != 0)
if (newList[r] == number1)
tt++;
if (tt == 0) {
newList[x] = number1;
t = true;
}
}
}
}

private static boolean copyAllUniqueNumbersIntoNewList(int[] list,
int[] newList) {
//what this loop does is check each member of "list" to see
//if it's a duplicate. Copy the current element to the newList
//if it's not a duplicate.
boolean dupLocated = false;
for (int b = 0; b <= list.length - 1; b++) {
int chkNum = list;
int temp = b;
boolean found = false;
for (int a = 0; a <= newList.length - 1; a++) {
if (newList[a] == chkNum) {
found = true;
dupLocated = true;
}
}
if (!found)
newList[temp] = chkNum;
}
return dupLocated;
}

private static void put6RandomNumbersIntoAnArray(int[] list) {
int number;
//put 6 random numbers into an array
for (int z = 0; z < 6; z++) {
Random rand = new Random();
number = rand.nextInt(49) + 1;
list[z] = number;
}
}

private static void initializeNewList(int[] newList) {
//initialize newList
for (int t = 0; t <= newList.length - 1; t++)
newList[t] = 0;
}

private static void initializeBigList(int[] BigList) {
//initialize BigList
for (int x = 0; x <= BigList.length - 1; x++)
BigList[x] = 0;
}
}
 
P

Print Guy

AndrewMcDonagh said:
snipped

ok, to be brutal - you have created a Procedural solution not an Object
Oriented solution. Given that Java is aimed at being an OO language
(lets not get into an argument about what that is nor Java's flaws) it
would be good to see the solution using the tools and techniques from an
OO perspective.

That said, even if we stick with a procedural solution your current one
needs a lot of work to improve its readability (never mind logic changes
to remove unneeded loops, conditions or state)
Brutal is good. My background is procedural languages even though I do
some scripting and attempt to employ a OO design technique.

What you are suggesting here is great. What I am really hoping for is
the implementation of my solution using classes and methods for objects
like

class bigListElement
int element;
int count;

void insert()
void delete()
void display()

But I thought that if I started out using the stuff I knew about I
would eventually be able to refine my design to the point where it was
a true Object Oriented program, not a Java translation of a Cobol
program :)

Thanks muchly for your help.... you and Patricia have given me alot to
think about.
 
L

Luke Webber

AndrewMcDonagh wrote:
[snip]
A simple starting point would be to turn comments into executable comments.

For example,

// the original array of 6 numbers picked
int [] list = new int[6];
// the new array containing unique numbers only
int[] newList = new int[6];
// this array is used to count the number of occurances (1-49)
int [] BigList = new int[50];

becomes something like...

int [] originalPickedNumbers = new int[6];
int [] uniqueNumbers = new int[6];
int [] numberOfOccurancesCount = new int[50];

Say what? Let's skip the spelling mistake for a minute here and ask one
simple question. Why "numberOf" AND "Count"? I mean, I like meaningful
names, but I'd go insane trying to work with code like that.

[snip]
private static int[] createNewListContainingOnlyTopSix(int[] BigList) { [snip]
private static void
incrementBigListValuesThatMatchEachOfTheSixNumbers(int[] list, int[]
BigList) { [snip]
private static void replaceAnyZeroSpaceHolderWithRandomNumber(int[]
newList) { [snip]
private static boolean copyAllUniqueNumbersIntoNewList(int[] list,
int[] newList) { [snip]
private static void put6RandomNumbersIntoAnArray(int[] list) {

With the greatest respect, this is taking things WAY too far.

Luke
 
A

AndrewMcDonagh

Luke said:
AndrewMcDonagh wrote:
[snip]
A simple starting point would be to turn comments into executable
comments.

For example,

// the original array of 6 numbers picked
int [] list = new int[6];
// the new array containing unique numbers only
int[] newList = new int[6];
// this array is used to count the number of occurances (1-49)
int [] BigList = new int[50];

becomes something like...

int [] originalPickedNumbers = new int[6];
int [] uniqueNumbers = new int[6];
int [] numberOfOccurancesCount = new int[50];

Say what? Let's skip the spelling mistake for a minute here and ask one
simple question. Why "numberOf" AND "Count"? I mean, I like meaningful
names, but I'd go insane trying to work with code like that.

I agree - but thats what the comments say the vars are ....

Once we rename them to be 'meaningful' i.e. get rid of the redundant
comment, we can start to look for other changes to the entire design.

The initial renaming is simply a starting point to make sure the usages
of the vars is more meaningful than 'list, biglist, newList' who's
purpose is as clear as mud unless you constantly refer back to where
they are defined.
[snip]
private static int[] createNewListContainingOnlyTopSix(int[] BigList) { [snip]
private static void
incrementBigListValuesThatMatchEachOfTheSixNumbers(int[] list, int[]
BigList) { [snip]
private static void replaceAnyZeroSpaceHolderWithRandomNumber(int[]
newList) { [snip]
private static boolean copyAllUniqueNumbersIntoNewList(int[] list,
int[] newList) { [snip]
private static void put6RandomNumbersIntoAnArray(int[] list) {

With the greatest respect, this is taking things WAY too far.

Why? Because the method names are long?
I type them once and let Eclipse auto-complete from then on. Longer,
more descriptive names help reveal and have a better chance of staying
up to date than comments.

Besides, you missed the main part, its a starting point.

The aim of these small refactorings are not to keep the methods, but to
reveal the algorithm used by the main() method.

Now we can see what the algorithm is within a few lines of code. We
can't nor need to see the implementation of the algorithm.

Once we have this ability to see the algorithm we are better placed to
refactor it further or rewrite it entirely.
 
A

AndrewMcDonagh

Print said:
Brutal is good. My background is procedural languages even though I do
some scripting and attempt to employ a OO design technique.

What you are suggesting here is great. What I am really hoping for is
the implementation of my solution using classes and methods for objects
like

Now that you have the first refactored version of your code, you can
start to look for duplication*.

Removing the duplication will create abstractions or behavior styled
methods. Once you have a number of these you should easily see that
there are logical groupings of the methods.

Once you see these groupings you can refactor further, by moving them
into their own class.


*duplication is not just code duplication - its also behavior or
algorithm duplication.

A quick review of the current design shows a number of Loops on lists,
doing something specific.

Each loop logic is the same. Its the specifics that different. These
specifics could either be extracted into a new class or the loop logic
called many times with different parameters for the loop to work on

for example...

we currently have.....

private static void initializeNewList(int[] newList) {
//initialize newList
for (int t = 0; t <= newList.length - 1; t++)
newList[t] = 0;
}

private static void initializeBigList(int[] BigList) {
//initialize BigList
for (int x = 0; x <= BigList.length - 1; x++)
BigList[x] = 0;
}

The duplication is obvious now we have these two methods.
And you should see that the only difference is the array being used.

so if we renamed one of the two methods above to 'initializeList(...)

we could simply call the same method twice, once with 'newList' and
another time with 'bigList'

as in....

public static void main(String args[]) {
Date today = new Date();
System.out.println("Starting:" + today);
//the original array of 6 numbers picked
int[] list = new int[6];
//the new array containing unique numbers only
int[] newList = new int[6];
int number = 0;
//this array is used to count the number of occurances (1-49)
int[] BigList = new int[50];

initializeList(BigList);

for (int y = 1; y <= 1000000; y++) {
initializeList(newList);

// snipped remaining...
}

private static void initializeList(int[] list) {
for (int x = 0; x <= list.length - 1; x++)
list[x] = 0;
}


class bigListElement
int element;
int count;

void insert()
void delete()
void display()

But I thought that if I started out using the stuff I knew about I
would eventually be able to refine my design to the point where it was
a true Object Oriented program, not a Java translation of a Cobol
program :)

Thats fine, but do keep in mind that its not a problem doing a
procedural approach per se.

The main problem comes should you need to use the solution in a program
where two different implementations of the Sample class could be chosen.

In other words, static functions don't support polymorphism, so a
program would not be able to accept/call a different version at runtime.

With your example, just moving the entire body of the 'main()' method
into an 'instance' method would allow the start of polymorphism to be
possible.

e.g.

class SampleRunner {

public void main(String[] args) {

Sample sample = new Sample();
sample.prinoutTopSixRandomNumbers();
}

}


class Sample {

public void printoutTopSixRandomNumbers() {
// insert code from your main() method here....
}

}





Thanks muchly for your help.... you and Patricia have given me alot to
think about.
np - glad to help...
 
J

jgrabell

import java.util.ArrayList;
import java.util.Random;
public class SixFourNine {
ArrayList winningNumbers = null;
public static int NUM_CHOICES = 6;
public static int MAX_NUMBER = 49;
int[][] simulationArray = new int[MAX_NUMBER][2];
/** creates a new SixFourNine object**/
public SixFourNine() {
initializeSimulationArray();
}
/** initializes the array for storing the simulation **/
public void initializeSimulationArray() {
for (int i=0; i <MAX_NUMBER; i++) {
simulationArray[0] = (i+1);
simulationArray[1] = 0;
}
}
/** default simulation -- 1 million trials **/
public void runSimulation() {
runSimulation(1000000);
}
/** simulation -- specify # trials **/
public void runSimulation(int numTrials) {
if (numTrials <= 0 ) return;
Random rand = null;
int number = 0;
for (int i = 0; i < numTrials; i++ ){
rand = new Random() ;
number=rand.nextInt(MAX_NUMBER) + 1;
simulationArray[number-1][1]++;
}
}
/** get the winning numbers. throws an exception if you have not
run a simulation **/
public ArrayList getWinningNumbers() throws
SixFourNineSimulationNotRunException {
if (simulationArray == null) throw new
SixFourNineSimulationNotRunException();
winningNumbers = new ArrayList();
for (int i = 0; i < NUM_CHOICES; i++) {
Integer r = new Integer(findAndRemoveMax());
winningNumbers.add(r);
}
initializeSimulationArray();
return winningNumbers /*NOT A GUARANTEE*/;
}
/** finds the most picked number from the simulations, in the case
of a tie, largest number (index) wins **/
private int findAndRemoveMax() {
int max = 0;
int count = 0;
for (int i = 0; i < MAX_NUMBER; i++) {
if (simulationArray[1] > count) {
count = simulationArray[1] ;
max = i;
}
}
simulationArray[max][1] =0;
return max+1;
}
/** program main **/
public static void main(String[] args) {
SixFourNine sfn = new SixFourNine();
sfn.runSimulation(1000000);
try {
System.out.println(sfn.getWinningNumbers());
} catch (SixFourNineSimulationNotRunException sfnnre) { // this
cannot happen in this example but we must catch;
System.out.println("Sorry, there was a problem; you need to
run a simulation before you can guess the winning numbers!");
System.exit(42);
}
System.exit(0);
}
public class SixFourNineSimulationNotRunException extends Exception
{};
}
 
D

Daniel Dyer

Hi all.

To most of you, what I have been working on is probably trivial and
easy, but it's taken me the good part of a week working 2 or 3 hrs a
day to finally figure out how to do what I needed to do.
Here in Canada, we have a lottery called 6-49. You pick 6 numbers
between 1 and 49. Those numbers are put on a ticket and there is a
draw twice a week.

I wanted to come up with a statistically solid way to pick my numbers
so I figured that if I were to pick 6 numbers 1,000,000 times and count
the number of times each number is selected, the top six would be good
numbers to bet on during the lottery.

How do you mean "statistically solid"? Unless you know of some flaw in
the machine that picks the numbers, any set of six numbers is equally
likely to come up as any other. You may as well pick 1, 2, 3, 4, 5 and 6
and be done with it. Unless, as Stefan suggested, you want to maximise
your winnings if you ever hit the jackpot, in which case you need data
about how often each number is selected by other players. In the absence
of any hard data, I would guess that picking high numbers is the way to go
as many people pick numbers that correspond to dates (birthdays,
anniversaries etc.), which are all in the range 1-31.
With that being said, I thought I could write a Java program to do the
job. It proved to be a bit more difficult than I thought it would be;
the hardest part being ensuring that each of the 6 numbers were unique.

java.util.Random is not very good at generating statistically random
numbers. See Roedy Green's page
(http://mindprod.com/jgloss/pseudorandom.html) for more details. An
improvement would be to use the SecureRandom class, which has a better
distribution but is a bit slower.

Dan.
 
D

Daniel Dyer

Why not run Fisher-Yates for 6 iterations, and then pick up the
first 6 elements of the array?

OK, but if you are going to generate 6 random numbers between 1 and 49 in
order to decide which elements to swap, why not just use those random
numbers as your selected lottery numbers?

Dan.
 
P

Patricia Shanahan

Daniel said:
OK, but if you are going to generate 6 random numbers between 1 and 49
in order to decide which elements to swap, why not just use those random
numbers as your selected lottery numbers?

Because they may contain duplicates, and the OP wants unique numbers.

Patricia
 
L

Luke Webber

AndrewMcDonagh said:
Luke said:
AndrewMcDonagh wrote:
int [] originalPickedNumbers = new int[6];
int [] uniqueNumbers = new int[6];
int [] numberOfOccurancesCount = new int[50];

Say what? Let's skip the spelling mistake for a minute here and ask
one simple question. Why "numberOf" AND "Count"? I mean, I like
meaningful names, but I'd go insane trying to work with code like that.

I agree - but thats what the comments say the vars are ....

So it's occurrenceCount or numOccurrences maybe, or even
numberOfOccurrences if you really must, but I'd prefer something a
little short, though still meaningful.
Once we rename them to be 'meaningful' i.e. get rid of the redundant
comment, we can start to look for other changes to the entire design.

The initial renaming is simply a starting point to make sure the usages
of the vars is more meaningful than 'list, biglist, newList' who's
purpose is as clear as mud unless you constantly refer back to where
they are defined.

True, no doubt. As we used to say in the "old days", "real programmers
can write FORTRAN in any language". said:
[snip]
private static int[] createNewListContainingOnlyTopSix(int[]
BigList) { [snip]
private static void
incrementBigListValuesThatMatchEachOfTheSixNumbers(int[] list, int[]
BigList) { [snip]
private static void replaceAnyZeroSpaceHolderWithRandomNumber(int[]
newList) { [snip]
private static boolean copyAllUniqueNumbersIntoNewList(int[] list,
int[] newList) { [snip]
private static void put6RandomNumbersIntoAnArray(int[] list) {

With the greatest respect, this is taking things WAY too far.

Why? Because the method names are long?
I type them once and let Eclipse auto-complete from then on.

Which doesn't work for variable names. And doesn't even work for methods
withing the current class unless you preface it with "this.", which is
still more typing. But it;s not just typing it in that concerns me. It's
/reading/ it as well. When the method names get that long, they actually
start to /lose/ meaning for me, because I kind of zone out on the
difference between "aReallyReallyLongNameThatGoesOnForever" and
"aReallyReallyLongNameThatGoesOnForeverAndADay". There is a happy middle
ground somewhere, where the names are meaningful, but not so long that
you have difficulty parsing them in your head.

For me, you don't have to be able to look at a method name and have it
tell you exactly what the method does. That's what documentation for. As
long as the method is sufficiently meaningful to give me a fairish idea
of its function, I'm frabjous.
> Longer,
more descriptive names help reveal and have a better chance of staying
up to date than comments.

And if you decide that you want to handle Powerball numbers and system 5
entries as well, then what? You have to change the method name AND ALL
CALLS TO IT, because it's no longer strictly six numbers?

And for that matter, why "IntoAnArray"? Your argument is an array, so
that tells the story right there.
Besides, you missed the main part, its a starting point.

Ah, but when you tell a new programmer that "this is a starting point",
they're likely to take it even /further/, and you never know where he'll
end up said:
The aim of these small refactorings are not to keep the methods, but to
reveal the algorithm used by the main() method.

Now we can see what the algorithm is within a few lines of code. We
can't nor need to see the implementation of the algorithm.

That's what Javadoc is for, IMO. I'm pleased to note that there are no
examples if this level of verbosity in the standard libraries.
Once we have this ability to see the algorithm we are better placed to
refactor it further or rewrite it entirely.

Maybe. OTOH, as I've said before, once you start rewriting those
methods, you're going to have to change the names to reflect their new
function. But if they were at least slightly more generic, you wouldn't
need to.

Luke
 
P

Print Guy

How do you mean "statistically solid"? Unless you know of some flaw in
the machine that picks the numbers, any set of six numbers is equally
likely to come up as any other. You may as well pick 1, 2, 3, 4, 5 and 6
and be done with it. Unless, as Stefan suggested, you want to maximise
your winnings if you ever hit the jackpot, in which case you need data
about how often each number is selected by other players. In the absence
of any hard data, I would guess that picking high numbers is the way to go
as many people pick numbers that correspond to dates (birthdays,
anniversaries etc.), which are all in the range 1-31.

I thought that picking 6 random numbers between 1 and 49 a million
times would be a reasonably good estimate of the real world odds. I
guess I was wrong.

Actually, the odds are that if you roll a dice, you will get a 3 or 4
more than any other number so I was trying to sort of simulate that.
 

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

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,483
Members
44,901
Latest member
Noble71S45

Latest Threads

Top