Mu3
Mu3

Reputation: 169

Setter sets value for every object in class

I have a program that takes students' names and grades as user input and then performs some operations on them, which are irrelevant for the scope of the question. The code is as follows:

import java.io.*;
import java.util.Arrays;
import java.util.Scanner;

public class Student {
// Four attributes that define Student
private String name;
private double points;
private int startYear;
private int[] grades;

public Student(String name, double points, int startYear, int[] grades) {
    this.name = name;
    this.points = points;
    this.startYear = startYear;
    this.grades = grades;
}
//Constructor. Everyone starts with 0 points and this year

public static void main(String[] args) {
    Scanner sc = new Scanner(System.in); //Create scanner
    System.out.println("Please enter the number of students:");
    int count = sc.nextInt(); // Number of students
    System.out.println("Please enter the number of grades:");
    int count1 = sc.nextInt(); // Number of grades
    Student students[] = new Student[count]; // Create array of student objects based on previously entered value
    int[] temp = new int[count1]; // Temporary array for storing grades entered
    for (int i = 1; i < count + 1; i++) {
        System.out.println("Please enter the name of student " + i);
        String name = sc.next();
        students[i - 1] = new Student(name,0.0,2018,temp); // Creating student object
        System.out.println("Please enter grades of  " + name);
        for (int k = 0; k < count1; k++) {
            int personal_grades = sc.nextInt();
            temp[k] = personal_grades; //filling the temporary array
            //System.out.println(grades.length); //for debugging

        }
        students[i - 1].setGrades(temp); //transferring from temporary array to student object array
        students[i-1].printGrades();

    }
    System.out.println((students[0].name));
    System.out.println((students[1].name));
    System.out.println(Arrays.toString(students[0].grades));
    System.out.println(Arrays.toString(students[1].grades));
    for(int i = 0; i < count; i++) {
        System.out.println("Grades of  " + students[i].name + " are:");
        //students[i].printGrades();
    }
    for(int i = 0; i < count; i++) {
        System.out.println("Average of  " + students[i].name + " is:");
       // students[i].average();
    }
    int passed=0;
    for(int i = 0; i < count; i++) {

        if(students[i].average()>5.5)
        {
            passed++;

        }

    }
    System.out.println(passed+" students passed!");


}

public void setGrades(int[] temp) {

        this.grades = temp;


}

public int[] getGrades() {
    return grades;
}

public void printGrades() {


        System.out.println(Arrays.toString(grades));

    }

public float average (){
    int k = 0;
    int sum=0;
    float average=0;
    while (k < this.grades.length) {
        sum=sum+this.grades[k];
       k++;

    }
    average = sum/(float)this.grades.length;
    System.out.println(average);
    return average;
}
}

The problem I am having with the code is as follows: the setter method appears to set the values for all of the objects that were ever created. Take this test run as an example: enter image description here

You can see that the grades for the last student entered appear in every student's record. I have debugged and found that it is the setGrades method that causes this. However, I am using the this keyword - why does it set the value for all the objects then?

Upvotes: 0

Views: 430

Answers (4)

Thiyagu
Thiyagu

Reputation: 17910

Move the array (temp) holding the grades inside the loop where you create individual Students

for (int i = 1; i < count + 1; i++) {
    ...
    int[] temp = new int[count1];  //The array holding the grades must be *specific* for each student

    students[i - 1] = new Student(name, 0.0, 2018, temp); // Creating student object
    ...
    students[i - 1].setGrades(temp); //transferring from temporary array to student object array
    students[i - 1].printGrades();

}

In your original code, you are using just one array i.,e temp was pointing to the same array all the time. After you finish initializing the first Student, when you loop populating the grades for the second student, you are mutating (or modifying) the same grades array created for the first student.

Upvotes: 0

GalAbra
GalAbra

Reputation: 5138

Note how both Student's constructor and Student::setGrades() get grades by reference.

This means that for each Student's instance, its grades field points to the parameter that was received during its initialization.

However, you only initialize temp once and therefore all the instances point to the same grades array. Once this array is changed, calling student.printGrades() will print the shared array's contents.

This can be solved by initializing temp on every iteration, before creating a new Student instance; Or by copying the array by value inside setGrades() method:

public void setGrades(int[] temp) {
    this.grades.clone(temp);
}

Upvotes: 0

paulturnip
paulturnip

Reputation: 106

It's because you are using the same array for all the grades of everyone.

Moving temp = new int[count1]; inside the first loop should fix it

Upvotes: 0

luk2302
luk2302

Reputation: 57174

You need to move the

int[] temp = new int[count1]; // Temporary array for storing grades entered

inside the outer for loop, otherwise all created Students will have a reference to same grades array and all will end up with the grades of the last student.

Upvotes: 1

Related Questions