Whats wrong with my "add" method?

[SpeedRunner]

Here's my whole program so far, not completed yet. It uses java.util.LinkedList. I just want to point out my "add" method.

import javax.swing.*;
import java.util.*;

public class RosterTest
{

   public static void main(String[] args)
   {
      String input;
      Roster myRoster = new Roster();
      do
      {
         input = menu();
         if(input == null || input.equalsIgnoreCase("q"))
         {
            JOptionPane.showMessageDialog(null,"Goodbye!");
         }
         else if(input.equalsIgnoreCase("a"))
         {
            doAdd(myRoster);
         }
         else if(input.equalsIgnoreCase("f"))
         {
            doFind(myRoster);
         }
         else if(input.equalsIgnoreCase("r"))
         {
            doRemove(myRoster);
         }
         else if(input.equalsIgnoreCase("l"))
         {
            JOptionPane.showMessageDialog(null,myRoster);
         }
         else
         {
            JOptionPane.showMessageDialog(null,"Invalid - try again!");
         }
      }while(input != null && !input.equalsIgnoreCase("q"));
      System.exit(0);
   }

   public static String menu()
   {
      String s = "MENU\na - add\nf - find\nr - remove"
         + "\nl - list\nq - quit\nEnter a letter:";
      return JOptionPane.showInputDialog(s);
   }

   public static void doAdd(Roster myRoster)
   {
      String name = JOptionPane.showInputDialog(
            "Enter student name:");
      int number = Integer.parseInt(JOptionPane
         .showInputDialog("Enter student number:"));
      double gpa = Double.parseDouble(JOptionPane
         .showInputDialog("Enter student gpa:"));
      boolean added = myRoster.add(new Student(name,number,gpa));
      if(!added)
      {
         JOptionPane.showMessageDialog(null,"Student with number " +
            number + " already exists!");
      }
   }

   public static void doFind(Roster myRoster)
   {
      int number = Integer.parseInt(JOptionPane
         .showInputDialog("Enter student number:"));
      Student foundStu = myRoster.find(number);
      if(foundStu == null)
      {
         JOptionPane.showMessageDialog(null,"No student with number "
            + number + "!");
      }
      else
      {
         JOptionPane.showMessageDialog(null,foundStu);
      }
   }

   public static void doRemove(Roster myRoster)
   {
      int number = Integer.parseInt(JOptionPane
         .showInputDialog("Enter student number:"));
      Student removedStu = myRoster.remove(number);
      if(removedStu == null)
      {
         JOptionPane.showMessageDialog(null,"No student with number "
            + number);
      }
      else
      {
         JOptionPane.showMessageDialog(null,"Removed student: " + removedStu);
      }
   }
}
//A collection of students, ordered by student number

/*Remember in particular that using the "enhanced for loop" will be very helpful.
You can use it to your advantage in find, remove, and toString.*/
class Roster
{
	 private List<Student> stuList = new LinkedList<Student>();

	 public Roster()
	 {
	 }
	 //Inserts the parameter student into the linked list,
	 // returning true if the student is actually added, false otherwise
    public boolean add(Student addStu)
    {
		 /*In your Roster add, check to see if your list "contains" the student to be added.
		 (This is why you overrode "equals" in Student.
		 If a student with that number is in the list,
		 then contains will return true, telling you that you must not add a student with that
		 number again. Otherwise, add the new student to the list*/
		 if (stuList.contains(addStu))
		 {
			 return false;
		 }
		 else
		 {
			 stuList.add(addStu);
			 return true;
		 }
	 }
	 //Finds a student with a particular number in the list
	 public Student find(int toFind)
	 {
		 /*Your roster find method can use the enhanced for loop,
		 cycling through all the students and looking for one whose number
		 matches the search number. If found, return that student.*/
		 for(int i =0; stuList.contains(toFind); i++)
		 {
		 }
		 return null;
	 }
	 //Removes from the list the student having the parameter student number.
	 public Student remove(int toKill)
	 {
		 /*For the remove method, use your own find method to get a reference to
		 the student to be removed.  If a non-null was obtained from find,
		 then use the List remove method to remove the student.
		 Return the reference to the student (or null, as the case may be).*/
		 return null;
	 }
	 //Returns a string representation of this student
	 public String toString()
	 {
		 System.out.println(stuList);
		 return null;
	 }
}
//This class represents a student in a Roster.
class Student
{
    private String name;
    private int number;
    private double gpa;

    //Constructs a new Student object
	 public Student(String nam, int num, double g)
	 {
		 name = nam;
		 number = num;
		 gpa = g;
	 }
    //Returns the student number of this student
    public int getNumber()
    {
		  return number;
	 }
	 public boolean equals(Student stu)
	 {
		 /*If "this" student has the same number as the parameter student,
		  then consider them equal.*/
		  if(this.getNumber() == stu.getNumber())
		  {
			  return true;
		  }
		  else
		  {
			  return false;
		  }
	 }
    //Returns a String representing the student's data
    public String toString()
    {
        return("Name " + name + ", ID" + number + ", GPA" + gpa);
    }
}

here's the add method:

    public boolean add(Student addStu)
    {
		 /*In your Roster add, check to see if your list "contains" the student to be added.
		 (This is why you overrode "equals" in Student.
		 If a student with that number is in the list,
		 then contains will return true, telling you that you must not add a student with that
		 number again. Otherwise, add the new student to the list*/
		 if (stuList.contains(addStu))
		 {
			 return false;
		 }
		 else
		 {
			 stuList.add(addStu);
			 return true;
		 }
	 }

The problem during runtime is that when a new entry is added, it should not allow me to add a duplicate entry. But it does. I don't know why...

 

[drizzt81]

I looked at you code and cannot find an obvious mistake. I think it's debugger time.

 

[ameoba]

When you say you're adding duplicate entries, what, exactly, do you mean by dupes?

It's most likely:
a) You're not calling the correct add()
b) you're not calling the correct equals()
c) you're compiling the wrong version of your source

 

[SpeedRunner]

What I'm saying is that in the add method, it should return false if the entry has already been added, but if it hasn't been added yet, to add it and return true. It's either not adding, or not correctly checking if the entry is already in there. I can't tell which.

 

[drizzt81]

What I'm saying is that in the add method, it should return false if the entry has already been added, but if it hasn't been added yet, to add it and return true. It's either not adding, or not correctly checking if the entry is already in there. I can't tell which.

run the program in the debugger, see what it does. That way you can tell which function is not working correctly. Add one student and then add him/ her again and see whether compare returns true. If yes, then you have a problem there, else you have a problem elsewhere.

 

[SpeedRunner]

Well... I'm using Textpad and I don't think it has a debugger.

 

[saxy_simon]

As suggested the 'compareTo' and 'equals' methods in Student are the problem. Or, to be more precise, the lack of them.

 

[Tawnos]

A bit off topic, but related to the number of java questions you're asking :

Speedrunner, what school are you attending? Comp Sci major? Data structures and algorithm analysis class?

 

[JasonE4]

Try changing Student's equals method to this:

                 public boolean equals(Object obj)
                 {
		 /*If "this" student has the same number as the parameter student,
		  then consider them equal.*/
                  
                  if(getClass() != obj.getClass()) return false;
                  Student stu = (Student)obj;
                  
		  if(this.getNumber() == stu.getNumber())
		  {
			  return true;
		  }
		  else
		  {
			  return false;
		  }
                  }

 

[SpeedRunner]

I'm in a Data Structures class. Jason, thanks, that made it work. Now I'm trying to figure out why that works...

 

[JasonE4]

Your equals method had a Student object as a parameter instead of an Object, and therefore it did not override Object's equal method (you "overloaded" the equals method).

 

[Tawnos]

Jason, that bit of code would have lost you a fair number of points in a couple of my professors' classes...

if(this.getNumber() == stu.getNumber())
		  {
			  return true;
		  }
		  else
		  {
			  return false;
		  }

should just be

	return this.getNumber() == stu.getNumber();

 

[JasonE4]

Oh, I wouldn't have done it like that either, but I just changed what was absolutely necessary to change in order to get it working.

Jason, that bit of code would have lost you a fair number of points in a couple of my professors' classes...

if(this.getNumber() == stu.getNumber())
		  {
			  return true;
		  }
		  else
		  {
			  return false;
		  }

should just be

	return this.getNumber() == stu.getNumber();

 

[Tawnos]

Ah, sorry, didnt' make it that far down in the original code to see you were copy pasting.

Speedrunner, then, this is for you:
Return values are almost always one liners.