Student Ranking

[AbstractEden]

One of my assignments, that I already turned in, dealt with collecting a student's name and gpa, and storing the information in an array. Afterwards, we were asked to display the student's information in descending order based on their gpa. I was curious if anyone has any suggestions on improving the code.

This is my student class:

/**
Define a class Student. Each student is defined by:
   * a string name
   * a floating-point GPA
*/

import java.util.*;
import java.text.DecimalFormat;

public class Student
{
   private String name;
   private double GPA;

   //to enter the data for a new student
   public Student()
   {
      Scanner keyboard = new Scanner(System.in);

      name = keyboard.nextLine();
      GPA = keyboard.nextDouble();
   }

   //to retrieve a student's data
   public String getName()
   {
      return name;
   }

   public double getGPA()
   {
      return GPA;
   }

   //to change data for a student
   public void changeName(String s)
   {
      name = s;
   }

   public void changeGPA(double g)
   {
      GPA = g;
   }

   //to display a student's information
   public void displayStudentInfo()
   {
      System.out.print("Name: "+name+"\t"+"GPA: "+GPA);
      System.out.println();
   }

}

And this is my test class:

public class Test
{
   //Calculate the average GPA of these students
   public static double averageGPA(Student[] arr)
   {
      double total = 0;
      double average;

      for(int i = 0; i < arr.length; i++)
      {
         total += arr[i].getGPA();
      }

      average = total / arr.length;

      return average;
   }

   //Find out the highest GPA and students' names
   public static double highestGPA(Student[] arr)
   {
      double grade = arr[0].getGPA();

      for(int i = 1; i < arr.length; i++)
      {
         if(arr[i].getGPA() > grade)
            grade = arr[i].getGPA();
      }
      return grade;
   }

   //Display the student ranking based on their GPAs in descending order
   public static void sort(Student[] arr)
   {
     double temp = arr[0].getGPA();
     String temp2 = arr[0].getName();

     for(int i = 0; i < arr.length; i++)
     {
         for(int j = 1; j < arr.length; j++)
         {
           if(arr[j-1].getGPA() < arr[j].getGPA())
           {
               temp = arr[j-1].getGPA();
               arr[j-1].changeGPA(arr[j].getGPA());
               arr[j].changeGPA(temp);

               temp2 = arr[j-1].getName();
               arr[j-1].changeName(arr[j].getName());
               arr[j].changeName(temp2);
           }
         }
     }
   }

   public static void main(String[] args)
   {
      //Create an array of 5 students
      Student[] info = new Student[5];

      //Enter data for 5 students
      for(int i = 0; i < info.length; i++)
      {
         System.out.print("Student #"+(i+1)+": ");
         info[i] = new Student();  
      }
      System.out.println();

      System.out.println("Average GPA: "+averageGPA(info));
      System.out.println("Highest GPA: "+highestGPA(info)+"\n");

      System.out.println("Student Ranking:\n");

      sort(info);
      for(int i = 0; i < info.length; i++)
         System.out.println("Name: "+info[i].getName()+"\t\t"+info[i].getGPA());
   }
}

[JamesCherrill]

In the sort, instead of swapping names and gpas, just swap the two Student objects. That's easier, and it won't break when you add another field, eg date of birth, to the student class. It's also very bad practice for a sort to change the values in the objects its sorting, suppose the Student data is backed by an SQL table, when you change the name or gpa you will trigger a database update!

[stultuske]

a few things I would change:

don't instantiate Scanner in the instance (constructor) itself. you'll create a new instance of Scanner for each and every instance of Student, which is a waste of resources.

just create a constructor that takes two parameters (String name, double gpa), and set the values like that. only instantiate the scanner in the calling class, that way, you'll only need one instance.

next: don't do a

public void displayStudentInfo()
   {
      System.out.print("Name: "+name+"\t"+"GPA: "+GPA);
      System.out.println();
   }

replace it by:

public String toString(){
  return "Name: " +name+"\t GPA: " + GPA);
  }

for now, your code 'll work just fine. but what 'll happen if the time comes you'll need to print the student's information in a Swing GUI? you'll need to change the class...