arrayHistogram implementation

Question

I wrote this code, called arrayHistogram, that would generate a random number within a set range, and print it in an array randomIntArray. It would then make another array arrayHist that would count how many times each number in randomIntArray occurred. The program works, but, I feel I have a few semantic errors in the syntax, that's not printing arrayHist correctly. I would like to review it and see where the errors are so I can correct them. Also, If you feel like you can offer me a more efficient methods, and means for this code, feel free to do so, I'll take it into consideration.

{
 public static void main(String[] args){
 int[] array=randomIntArray(30);//declares the dimensions of the array "randomIntArray"
 System.out.println("Scores:");
 printArray(array);//calls the scoreHist method to print the "array variable
 System.out.println("\n"+"count: ");
 printArray(arrayHist(array));//calls score hist, to call arrayHist, to count "array"
 System.out.println();
 }
 public static int randomInt(int low, int high){//the prng
 double e;
 double x=Math.random();
 e=low+x*(high-low);
 return (int)e;}
 public static int[] randomIntArray(int n){//generates an array of random numbers based on an upper and lower bound
 int[] a=new int[n];
 for(int i=0;i<a.length;i++)
 {a[i]=randomInt(-5,15);}//"-5&15 are the lower and upper bounds of the random number array
 return a;}
 public static int inRange(int[] a, int low, int high){//checks if the numbers of the random number array are in certain constraints.
 int count=0;
 for(int i=0;i<a.length;i++)
 {if(a[i]>=low&&a[i]<=high)count++;}
 return count;}
 public static int[] arrayHist(int[] scores){//counts how many of each number occurs in "randomIntArray"
 int[] counts=new int[11];
 for(int i=0;i<10;i++)
 {counts[i]=inRange(scores,i,i++);}
 return counts;}
 public static void printArray(int[] a){//prints any array, with only one traversal.
 for(int i=0;i<a.length;i++)
 {System.out.print(" "+a[i]);}
 }
}

My goal is to not use any of the array methods Java provides (to challenge myself), so please don't offer them up.

• Legato: Look at this: uncyclopedia.wikia.com/wiki/Wall_of_Text. Then look at your code.
• theUser: Ah, I see now. How can I avoid this int the future, as, I am knew to stackExchange
• Legato: Do what Lyle suggested and add more spaces between the lines in your code.

Answer 1

you need to add some spaces in between operators and format the code properly

all I did here was

• added spaces between operators
• moved the curly braces to their appropriate locations for Egyptian Style Bracing
• set indentation to standard, you had some lines that were indented and they shouldn't have been

public static void main(String[] args) {
 int[] array = randomIntArray(30); //declares the dimensions of the array "randomIntArray"
 System.out.println("Scores:");
 printArray(array); //calls the scoreHist method to print the "array variable
 System.out.println("\n" + "count: ");
 printArray(arrayHist(array)); //calls score hist, to call arrayHist, to count "array"
 System.out.println();
}
public static int randomInt(int low, int high) { //the prng
 double e;
 double x=Math.random();
 e=low+x*(high-low);
 return (int)e;
}
public static int[] randomIntArray(int n) { //generates an array of random numbers based on an upper and lower bound
 int[] a = new int[n];
 for(int i = 0; i < a.length; i++) {
 a[i] = randomInt(-5, 15);
 } //"-5&15 are the lower and upper bounds of the random number array
 return a;
}
public static int inRange(int[] a, int low, int high) { //checks if the numbers of the random number array are in certain constraints.
 int count = 0;
 for(int i = 0; i < a.length; i++) {
 if(a[i] >= low && a[i] <= high) count++;
 }
 return count;
}
public static int[] arrayHist(int[] scores) { //counts how many of each number occurs in "randomIntArray"
 int[] counts=new int[11];
 for(int i=0;i<10;i++) {
 counts[i] = inRange(scores, i, i++);
 }
 return counts;
}
public static void printArray(int[] a) { //prints any array, with only one traversal.
 for(int i = 0; i < a.length; i++) {
 System.out.print(" " + a[i]);
 }
}

---

I noticed that you are using a For loop, and I think that you should be using more foreach loops. I am going to use this code as an example

public static int inRange(int[] a, int low, int high) { //checks if the numbers of the random number array are in certain constraints.
 int count = 0;
 for(int i = 0; i < a.length; i++) {
 if(a[i] >= low && a[i] <= high) count++;
 }
 return count;
}

so what you get is now this

public static int inRange(int[] a, int low, int high) {
 int count = 0;
 for(int i : a) {
 if(i >= low && i <= high) count++;
 }
 return count;
}

which is slightly cleaner than the OP.

I also removed the comments on this method, mainly because they were unnecessary. you have a lot of comments in this code some of which are not needed at all, probably all of them. most of your variables are named appropriately so you don't have a doubt what they stand for. nothing is too complex here that you would need to explain what is going on.

(削除) here are the comments I would get rid of

• //declares the dimensions of the array "randomIntArray"
• //declares the dimensions of the array "randomIntArray"
• //the prng (削除ここまで)

Here is the only comment that I would keep

//counts how many of each number occurs in "randomIntArray"

on the ArrayHist Method. because the Method name doesn't really tell me that right away.

---

All Methods should be named in PascalCase, variables in camelCase.

Answer 2

I'm not sure, that arrayHist method does what it's intended to do.

public static int[] arrayHist(int[] scores)//counts how many of each number occurs in "randomIntArray"
{
 int[] counts = new int[11];
 for (int i = 0; i < 10; i++) {
 counts[i] = inRange(scores, i, i++);
 }
 return counts;
}

What worries me here, is that you increment counter i inside for loop body. Even if it was intended, I still suggest you to avoid this technique, because it complicates understanding of logic.
Also, I dared to rewrite this method, so that it builds histogram correctly (according to my understanding of course).

//counts how many of each number occurs in "randomIntArray"
public static int[] arrayHist(int[] scores)
{
 int[] counts = new int[NUMBER_OF_POSSIBLE_VALUES];
 for (int score : scores) {
 counts[score - LOWER_BOUND]++;
 }
 return counts;
}

I moved -5 and 15 out of randomIntArray function, and suggest you to do the same. Now they are constants called LOWER_BOUND and UPPER_BOUND respectively. I also initialize constant called NUMBER_OF_POSSIBLE_VALUES that is equal to

Math.abs(LOWER_BOUND) + (Math.abs(UPPER_BOUND)-1) + 1;

I decrement UPPER_BOUND modulus, because it's exclusive in randomInt; and add 1 to count zero.
Ok, back to arrayHist method. Now it initializes counts as array of zeros with size equal to number of every possible value that could be in your random int array. Now you should think that indexation of count starts not from 0 (zero), but from lower bound of range. After loop you get number of occurrences for those possible values from your range.

Surely, this code works nice as far as you don't change bounding values. If you will make them too big, it could cause overflow or OOM during counts allocation (in which case, you would probably want to switch from array to something like map).