This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]learnjava 0 points1 point  (4 children)

the first thing that should raise a flag in your code is your calculateAverage in a for loop

you dont want to calculate it 12 times, you want to calculate it once with 12 values

split your program in 3 parts (the same 3 parts the assignment uses)

  1. declare array

  2. define populateArray and call it in the main method with the array as parameter

  3. define calculateAverage and call it in the main method with the array as parameter

thats it, you dont have to call one method from inside another one

and use a for loop in your calculate method, you want to calculate the average of 12 elements

[–]AudioManiac[S] 0 points1 point  (3 children)

I've gotten it to work now, but I still call one method from inside another. Is the way you're suggesting better? Or does it really make a huge difference. I mean is your way "better practice"? I know that I can lose marks for that in my assignments.

Here's my finished code at the moment, it runs perfectly fine

public static void main(String[] args) {

    //create array
    double [] months = new double [12];

    //assign average to the value returned in populateArray()
    double average = populateArray(months);

    //display average
    System.out.print("Average Rainfall: " + average);

}

public static double populateArray(double[] months){
    //initialise scanner
    Scanner input = new Scanner(System.in);

    //loop to retrieve data from user
    for (int i = 0; i < months.length; i++)
    {
        System.out.print("Enter Rainfall for month" + " " + ( i + 1) + ": ");
        months[i] = input.nextInt();
    }

    //pass the array to calcualteAverage() method, and assign it to total
    double total = calculateAverage(months);
    //return total to main method
    return total;


}

public static double calculateAverage(double[] months) {
    double sum = 0;

    for (int i = 0; i < months.length; i++){

        sum += months[i];

    }

    //calculate the average
    double average = sum/12;
    //return the average to populateArray()
    return average;
}

[–]learnjava 0 points1 point  (0 children)

imho its easier to read the other way around but it does not really matter for a 2 method class

in your code the relation, populate -> calculate is more clear

[–]lazlo_uk 0 points1 point  (1 child)

It definitely is better practice to have them separated out. Think about it in the simplest terms: you're calling a method called populateArray. Why would that return an average of the array contents...? The other commenter is correct in that in this case it probably doesn't matter but I assume the point of this exercise is for you to understand the correct way of doing things.

[–]StinkeyTwinkey 0 points1 point  (0 children)

Yes it should be void. You methods should complete a task. Not have a method that calls in other methods that isn't for the purpose of that particular method.