New with coding block, one last piece

[Karl Beaudry]

I just need some assistance, My issue is in the output:
It reads: The following day has 74 people:

I can't figure out how or where i need to add the day part into the code so when the max or min loop back in to find the values it will also remember the day that it found it on. I have had this code written like this for three days, and I can't seem to get it past this point. Of course the output code needs to be cleaned up a bit, way it is written isnt the best.

Sorry for the way i put this in here too (if you have a better or cleaner way let me know) this is my first post ever to a forum like this. This is a assignment question, and I know people don't really like answering these. Its hard to ask my professor since I am three states away and work, usually by the time i can't figure something out its too late to email takes anywhere from 48hrs + for a response. This code has me at my end for that final piece. Any help or guidance on what i may be missing would be awesome.

 Random randomNumbers = new Random ();
    int Birth = 18975;
    int Day = 365;
    int f[] = new int [ Day + 1 ];

    for (int birth = 1; birth <= Birth; birth ++){
        int day = 1 + randomNumbers.nextInt (Day);
        f[day] ++;
    }
    for (int x = 1; x < f.length; x ++){
    System.out.printf ("Day%4d: %10d\n", x, f[x]);
    }
    int max = getMax(f);
    System.out.print("The Following day has "+max);
    System.out.println(" people: ");
    int min = getMin(f);
    System.out.print("The Following day has "+min);
    System.out.println(" people: ");
    }

public static int getMax (int[] inputf){
    int maxValue = inputf[0];
    for(int i=1; i < inputf.length; i ++){
        if(inputf[i] > maxValue){
            maxValue = inputf[i];
        }
    }
return maxValue;
}

public static int getMin(int[] inputf){
    int minValue = inputf[1];
    for (int i=1; i < inputf.length; i ++){
        if (inputf[i] < minValue){
            minValue = inputf[i];
    }
}
return minValue;
}
}

(Moderator edit: please UseCodeTags (←click) when posting code; it's more readable that way)

[Guillermo Ishi]

I'm not sure what you're doing but there are probably date structures in Java that would help. When I work with dates I usually store or calculate the number of seconds since some fixed date; 1/1/1970 is typical. You can use that to calculate year, day, day of week and so on without having to store and manage all that. You just retrieve or calculate the seconds and convert that at the end to whatever you need.

[Carey Brown]

Karl Beaudry wrote:I can't figure out how or where i need to add the day part into the code so when the max or min loop back in to find the values it will also remember the day that it found it on.

If I read you correctly, your "day" would be the index into the array. You could set up a variable maxIndex and initialize it to the index of the first valid data in the array, then, as you loop through the array, when the value is > maxValue, re-save the maxIndex. Note that you'd need to return the maxIndex instead of maxValue because the caller can deduce the value from the index but not the other way around.

[Junilu Lacar]

Also, why is line 32 different from line 22? Why do you skip the first array element (at index == 0) when you are looking for the minimum?

[Junilu Lacar]

And welcome to the Ranch!

[Karl Beaudry]

Thanks everyone for the reply's
So to answer a few questions. Basically what I am asking is when my code searches and finds the max and min value, I need it to store the day that it found the value on. So if the max is 73 on day 344, its already finding the max 73 but its not storing the day 344. Basically because I don't have that part written into it. I guess i can do an index, but would that be a whole new variable and initialized as well and then link it to the max and min array respectively. So could I set an index and grab the value (max or min) from the index, is that what you are saying.

Line 32 is different from line 22. When i set line 32 to a 0 (same as line 22), the min output is always zero, so i had to set it to 1. But i should have set line 22 to 1 as well, so i fixed that.

So here is the output that i am currently seeing (part of it). Just so you can see.

Day 363: 54
Day 364: 54
Day 365: 55
The Following day has 73 people:
The Following day has 26 people:

[Junilu Lacar]

Karl Beaudry wrote:So to answer a few questions. Basically what I am asking is when my code searches and finds the max and min value, I need it to store the day that it found the value on. So if the max is 73 on day 344, its already finding the max 73 but its not storing the day 344. Basically because I don't have that part written into it. I guess i can do an index, but would that be a whole new variable and initialized as well and then link it to the max and min array respectively. So could I set an index and grab the value (max or min) from the index, is that what you are saying.

The algorithm is basically the same, except instead of just maxValue/minValue, you need to track the value of the maxIndex/minIndex every time you find a new min/max value. Then you return the index instead of the max value. Since you already have the array, you can use the index returned to access the value that you are interested in.

To put it in more concrete terms, the "conversation" is something like this:

Q: Which element of this array has the maximum value?
A: The 32nd element; that is, the element where the index is 31 -- (this is what the min/max method will return)

After that, since you have the array:
Q: Hmm, I wonder what the value of the 32nd element is...
A: Oh, it's the value in array[31]!

Line 32 is different from line 22. When i set line 32 to a 0 (same as line 22), the min output is always zero, so i had to set it to 1. But i should have set line 22 to 1 as well, so i fixed that.

That would be wrong. Just try putting an very large number or very small number in element[0] and you will see why. You need to start with the assumption that the first element (index == 0) is the smallest/largest value in the array. Then as you iterate the array, correct that assumption when appropriate. So you need to initialize your min/max variable to array[0] before you go into the for-loop. (Edit: see follow-up response below. Leaving original comment to show that using 1-based logic in Java can confuse other developers)

[Junilu Lacar]

Ok, I see what's happening now. The reason you had the bug for getMin in the first place was because of this:
for (int birth = 1; birth <= Birth; birth ++){ int day = 1 + randomNumbers.nextInt (Day); f[day] ++; }
This code will always leave the first element of the array as 0 since you are adjusting the indexes to be 1-based instead of 0-based.

So, yes, you can adjust your other code so that it's 1-based or adjust this code so it is 0-based.

Most Java code idioms involving indices use/assume 0-based logic so writing code that goes against the grain of this, i.e. using 1-based index logic instead, can make your code confusing. If other developers are maintaining code like this, there is a higher chance of them introducing bugs and/or your code confusing them.

[Carey Brown]

Junilu Lacar wrote:

Line 32 is different from line 22. When i set line 32 to a 0 (same as line 22), the min output is always zero, so i had to set it to 1. But i should have set line 22 to 1 as well, so i fixed that.

That would be wrong. Just try putting an very large number or very small number in element[0] and you will see why. You need to start with the assumption that the first element (index == 0) is the smallest/largest value in the array. Then as you iterate the array, correct that assumption when appropriate. So you need to initialize your min/max variable to array[0] before you go into the for-loop.

Normally I'd agree with you that index 0 is the way to go, however Karl has implemented an array where data starts at index 1. I might suggest to Karl that he refactor the code at some point so that the whole thing is based from index of 0 to 364.

[Junilu Lacar]

Carey Brown wrote:
Normally I'd agree with you that index 0 is the way to go, however Karl has implemented an array where data starts at index 1. I might suggest to Karl that he refactor the code at some point so that the whole thing is based from index of 0 to 364.

Yup, I saw that, hence my followup reply

[Karl Beaudry]

Ok I see what you are saying, so write in a index so that it tracks the index value each time the code cycles (loops) to find the max/min. Thanks for the clarification, see what i come up with.

[Karl Beaudry]

Well I gave it my best shot for the last 4 1/2hrs, but i still can't get it to track the day for the max/min value. Well see what i get back, had to turn it in as is, if i turn it in with the wrong output at least get something vs nothing. Thanks for the help, I understand the concept and what i needed to do, just actually implementing it into the code just was not working for me, probably cause I am trying to implement it after the fact and im learning which usually doesn't mix. I tried different methods, and using different variables to track the day, but it continuously gave me wrong results, or wouldn't compile.

[Karl Beaudry]

Here was the cleaned up version minus the indexing. If you have any pointers to actually make it look cleaner, or flow better let me know. I know one of the biggest issues the guys I work with tell me is to make sure it flows and cleans up well, plus document document document on changes and methods.

import java.util.Random;
public class kbeaudry_Birthdays{
public static void main (String args[]){

/**
  * This Method is setting the 365 days and generating random numbers
*/
    Random randomNumbers = new Random ();
    int Birth = 18975;
    int Day = 365;
    int f[] = new int [ Day + 1 ];

    for (int birth = 1; birth <= Birth; birth ++){
        int day = 1 + randomNumbers.nextInt (Day);
        f[day] ++;
    }
    for (int x = 1 ; x < f.length; x ++){
    System.out.printf("Day%4d: %10d\n", x, f[x]);
    }
/**
  * Intializing the max and min values
*/

    int max = getMax(f);
    int min = getMin(f);
    System.out.printf("The Following has %d people:\n",max);
    System.out.printf("The Following has %d people:\n",min);
}
/**
  * This Method gets the max value of the random number generated
*/
public static int getMax(int[] inputf){
    int maxValue = inputf[0];
    for(int i=1; i < inputf.length; i ++){
        if(inputf[i] > maxValue){
            maxValue = inputf[i];
        }
    }
return maxValue;
}
/** This Mehtod gets the min value of the random numbers generated
*/
public static int getMin(int[] inputf){
    int minValue = inputf[1];
    for (int i=1; i < inputf.length; i ++){
        if (inputf[i] < minValue){
            minValue = inputf[i];
    }
}
return minValue;
}
}

[Campbell Ritchie]

Thkere must be a way to get that maximum with a Stream.
Let's look. The Arrays class has a method which takes an int[] and returns an IntStream object.
IntStream has a method which returns the maximum, as an OptionalInt, so you can get the value from that. If you pass an empty array you get an Exception. Only works in Java8.
System.out.println(Arrays.stream(myArray).max.getAsInt());

[Campbell Ritchie]

And my regular readers have already seen that you can get a Stream from a Random object. Look here to see how you can populate your random array from a Stream. The Random#ints method is overloaded and will allow you to specify a range of permitted values in the output.

[Campbell Ritchie]

A few minutes ago, I wrote: . . . my regular readers . . .

I hope I have some readers

[Darryl Burke]

Campbell Ritchie wrote:If you pass an empty array you get an Exception. Only works in Java8.
System.out.println(Arrays.stream(myArray).max.getAsInt());

Pass an empty array and return 0, not get an Exception:

Arrays.stream(intArray).max().orElse(0);

Campbell Ritchie wrote:I hope I have some readers

I can vouch foir one

[Junilu Lacar]

Karl Beaudry wrote:Here was the cleaned up version minus the indexing. If you have any pointers to actually make it look cleaner, or flow better let me know.

Ok, you asked for it...

First off, format the code to some kind of standard. I have reformatted your code below using the Eclipse code formatting feature (CTRL+SHIFT+F). Proper formatting and judicious use of whitespace helps clarify the intended flow, just as paragraphs and sentences help break up regular text into related sections.

import java.util.Random;

public class kbeaudry_Birthdays {

	public static void main(String args[]) {

		/**
		 * This Method is setting the 365 days and generating random numbers
		 */
		Random randomNumbers = new Random();
		int Birth = 18975;
		int Day = 365;
		int f[] = new int[Day + 1];

		for (int birth = 1; birth <= Birth; birth++) {
			int day = 1 + randomNumbers.nextInt(Day);
			f[day]++;
		}
		for (int x = 1; x < f.length; x++) {
			System.out.printf("Day%4d: %10d\n", x, f[x]);
		}
		/**
		 * Intializing the max and min values
		 */

		int max = getMax(f);
		int min = getMin(f);
		System.out.printf("The Following has %d people:\n", max);
		System.out.printf("The Following has %d people:\n", min);
	}

	/**
	 * This Method gets the max value of the random number generated
	 */
	public static int getMax(int[] inputf) {
		int maxValue = inputf[0];
		for (int i = 1; i < inputf.length; i++) {
			if (inputf[i] > maxValue) {
				maxValue = inputf[i];
			}
		}
		return maxValue;
	}

	/** This Mehtod gets the min value of the random numbers generated */
	public static int getMin(int[] inputf) {
		int minValue = inputf[1];
		for (int i = 1; i < inputf.length; i++) {
			if (inputf[i] < minValue) {
				minValue = inputf[i];
			}
		}
		return minValue;
	}
}

[Junilu Lacar]

Karl Beaudry wrote:I know one of the biggest issues the guys I work with tell me is to make sure it flows and cleans up well, plus document document document on changes and methods.

Code that is clean and flows well is good. Document Document Document, well, that depends on how you do it. JavaDocs are good but the style you use doesn't conform to the preferred style recommended for JavaDocs: http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#styleguide

Instead of

/**
 * This Method gets the max value of the random number generated
 */

Prefer something like this:

/**
 * Gets the index of the maximum value in the specified array. If the
 * array has more than one element that is equal to the greatest 
 * value in the array, the index of the first element with that value
 * will be returned.
 * 
 * @param a the array to be searched
 * @return the index of the first element containing the greatest
 *         value in the specified array.
 * @throws NullPointerException if the specified array is null
 */

I changed the documentation to reflect what we were advising you to do. Notice that I have also taken cues from the JavaDocs of the standard JDK API: http://docs.oracle.com/javase/8/docs/api/java/util/Arrays.html , in particular, the name of the parameter to the method.

Notice also that I removed "the random number generated" from your comment. I did that because if you look at just the method itself, there is nothing in there that indicates that the values in the array were randomly generated. The documentation for a method should not mention anything about things that are outside the purview of that method. The fact that you happen to be passing an array that has randomly generated values is not a concern of the method.

[Junilu Lacar]

Now let's look at this:
public static void main(String args[]) { /** * This Method is setting the 365 days and generating random numbers */ Random randomNumbers = new Random(); int Birth = 18975; int Day = 365; int f[] = new int[Day + 1];
Having a lot of code in main, as we say around here, is a pain. Click on that link and read what we recommend.

Following our recommendation, the main method would simply become:

public static void main(String[] args) {
    Birthdays birthdays = new Birthdays();
    birthdays.run(args);
}

I have taken the liberty of renaming the class here to follow standard naming conventions. That document is marked as outdated but it's still relevant with regard to naming. I also like to refer to Google's Java Style Guide

The comment you have there on lines 7-9 is a JavaDoc comment because it starts with a "/**". A JavaDoc comment like this goes right above the method, which in this case is main which I don't particularly see a need to document, so I would just delete this comment or move it above the class. Again, I would take cues from the Standard Library JavaDocs.

Next, on line 10 you declare the variable randomNumbers with default access level, also known as "package private" access. Nothing outside this class needs to access it so you should decrease the level to private and make it an instance variable or a local variable in an instance method. Right now it's a local variable in the static main method. The same thing goes for the int[] f variable that you declare on line 13.

Next, on line 11 you declare the int Birth variable and assign it a value of 18975. First of all, the name does not really fit the usage of the value in the program. The value is used to determine your sample size. Also, the value is not changed at all in the program, i.e., it's constant. To declare constants, we would make it a private static final member of the class and use all caps for the name. Same thing goes for the int Day variable declared on line 12.

To correct all of the above, your code would be refactored to something like this:

/**
 * The {@code Birthdays} class simulates a random sample population of birthdays
 * throughout the year and displays which days have the most and fewest birthdays 
 * occurring on them, respectively.
 *
 * @author Kevin B
 */
public class Birthdays {

    /* Constants */
    private static final int SAMPLE_SIZE = 18975;
    private static final int DAYS_IN_YEAR = 365;
   
    /* Instance variables */
    private Random randomGenerator = new Random();
    private int[] samples = new int[DAYS_IN_YEAR];

    public static void main(String[] args) {
        Birthdays birthdays = new Birthdays();
        birthdays.run(args);
    }
 
    private void run(String[] args) {
        ...       
    }    
}

I added some non-JavaDoc comments on lines 10 and 14. Next, we'll look at what would go in the run method.

[Junilu Lacar]

Let's start with the code that you wrote and put that in the run() method:
private void run(String[] args) { for (int birth = 1; birth <= Birth; birth++) { int day = 1 + randomNumbers.nextInt(DAYS_IN_YEAR); samples[day]++; } for (int x = 1; x < samples.length; x++) { System.out.printf("Day%4d: %10d\n", x, samples[x]); } /** * Intializing the max and min values */ int max = getMax(samples); int min = getMin(samples); System.out.printf("The Following has %d people:\n", max); System.out.printf("The Following has %d people:\n", min); }
This jumps right into the thick of things. Any other person reading this will be forced to buckle down and start analyzing this detailed code and try to figure out what it's doing. It may not seem like such a big deal but I prefer a gentler approach. See my comments in this thread: https://coderanch.com/t/647372/java/java/random-card-generator#2982605

With "be kind to the reader" in mind, I would lay out the high-level "story" for this program, like so:
private void run(String[] args) { generateRandomSamples(); displaySamplePopulation(); displayMostBirthdays(); displayFewestBirthdays(); }
Now it takes the reader only a couple of seconds to get an overall idea of what this program does. Writing it this way from the top down also gives you, the programmer, a way to organize your thoughts and break the problem down into smaller, more manageable pieces. This process is called decomposition.

[Junilu Lacar]

Notice, too, that the story told by the refactored code in the run method is relatively consistent with the JavaDoc comment we wrote for the class. This is a little redundancy I like to keep around as a sanity check. If the JavaDocs and the code are telling me the same thing, I usually tend to have more confidence that other comments are being kept up-to-date as well. The only detail we left out of the JavaDoc comment was the fact that the program also displays the random sampling before it displays the min/max birthday occurrences. We'll go with what we have for now.

[Campbell Ritchie]

Darryl Burke wrote: . . .
Pass an empty array and return 0, not get an Exception:

Arrays.stream(intArray).max().orElse(0);

Never knew about that. Thank you But wouldn't you pass Integer.MIN_VALUE as an argument to that method?

Campbell Ritchie wrote:I hope I have some readers

I can vouch foir one

Thank you

[Darryl Burke]

Campbell Ritchie wrote:wouldn't you pass Integer.MIN_VALUE as an argument to that method?

I guess that would be more appropriate, yes.

[Junilu Lacar]

Karl Beaudry wrote:I understand the concept and what i needed to do, just actually implementing it into the code just was not working for me, probably cause I am trying to implement it after the fact and im learning which usually doesn't mix. I tried different methods, and using different variables to track the day, but it continuously gave me wrong results, or wouldn't compile.

That happens sometimes. I usually take a break to take my mind off of the problem for a little bit and let my subconscious mind work on it in the background. If that still doesn't work, someone with a fresh pair of eyes can usually help out. Here's what we were telling you to do:

    /**
     * Finds the element with the greatest value
     * and returns its index in the specified array.
     * If there are multiple elements that contain
     * the maximum value, returns the index of 
     * first one.
     *
     * @param a the array to search
     * @return the index of the first occurrence
     *       of the greatest value in the specified array
     */
    public static int getMaxIndex(int[] a) {
        int index = 0;
        int max = a[0];
        for (int i = 1; i < a.length; i++) {
            if (a[i] > max) {
                index = i;
                max = a[i];
            }
        }
        return index;
    }

[Junilu Lacar]

Carey Brown wrote:... Karl has implemented an array where data starts at index 1. I might suggest to Karl that he refactor the code at some point so that the whole thing is based from index of 0 to 364.

The only part of the program that really needs to be adjusted to use 1-based indices is where you want to display the results. Rather than "Day 0 has the most number people with birthdays: ...", you'd want "Day 1 has the most ..." All other logic can and should stick to the normal 0-based idiom used in most Java code. This will minimize confusion and having to adjust the index every time you work with the array.

private void displayMostBirthdays() {
    int mostBirthdays = getMaxIndex(samples);
    System.out.printf("Day %d has the most number of birthdays celebrated: %d%n", 
        mostBirthdays + 1, 
        samples[mostBirthdays]); 
}

Here, you adjust the index by 1 in only one spot: the value of the day being displayed.

You can refactor one step further and extract the formula, mostBirthdays + 1, to its own separate method:

private void displayMostBirthdays() {
    int mostBirthdays = getMaxIndex(samples);
    System.out.printf("Day %d has the most number of birthdays celebrated: %d%n", 
        dayWith(mostBirthdays), 
        samples[mostBirthdays]); 
}

/** 
 * Converts the given 0-based index to a 1-based index
 * so that day in a year can be more naturally referred to 
 * with number from 1-365.
 */
private int dayWith(int d) {
    return d+1;
}

Refactoring this way may seem extreme but I actually find that line 3 is easier to read because the expression passed, dayWith(mostBirthdays), makes the code more conversational and I am consistently seeing the one value, mostBirthdays, rather than two slightly different ones: mostBirthdays + 1 and mostBirthdays. If I want to know what the method dayWith is about, I look at its JavaDoc and the detailed code that it encapsulates and that quickly clarifies the intent.

The dayWith method can then be reused in the displayFewestBirthdays() method, which will have a similar structure to the displayMostBirthdays method. With Java 8, you can use lambdas to make the code even tighter and more elegant.

[Karl Beaudry]

Wow, Thanks guys for all the posts. I'll go through these tonight and work on my coding. I apologize I was out of country for the last week, and just returned last night.

[Karl Beaudry]

Thanks for the coding advise to make things look cleaner, I will work on that. I just got this code working properly now thanks to your glorious advice with the indexing. While I was on the rode I was also researching to see if a linsearch/binsearch would work too. Got into my books and pdfs, but was unable to successfully implement the lin/bin searching. Finally got back and saw all the comments and advice.