[parry2411]

I have a Linked List which holds Student objects, On the Register class i have a Linked List that will hold the students, But i want a method that would allow me to delete a student from the list using their StudentIDNumber.

 public void removeStudent(int studentID)
    {
        for(Student pupil : studentList) 
        {
            if(pupil.getStudentID().equals(studentID)) 
            {
                studentList.remove(pupil);
                break;
           }
        }
        
    }

What am i doing incorrect here?

Thanks

[macosxnerd101]

What specific problems or errors are you encountering?

[parry2411]

No errors, Just that the Element that i want to delete does not get deleted, I can see where the problem leads to but im not sure on how to rectify it,

It loops through the list and looks on each object in the list, Then it checks to see if any objects getStudenbtID are equal to the student Id Entered.

This is where the problem i think is as it cant see the ID of the objects in the list.

[pbl]

parry2411, on 18 March 2013 - 11:50 AM, said:

This is where the problem i think is as it cant see the ID of the objects in the list.

If you can't see the ID it would mean that getStudentID() does not compile and you would have a compilation error

[parry2411]

The get method works fine

[CasiOo]

Your loop is kinda inefficient and will fail
Don't remove from the list while enumerating over it using a for-loop
If you use a list iterator, you will be able to remove items from the list using the built in methods provided by the iterator

public void removeStudent(int studentID)
   {
       for(Student pupil : studentList) 
       {
           if(pupil.getStudentID().equals(studentID)) 
           {
               studentList.remove(pupil); //<- Concurrency problems right here. Also you make it search for 'pupil' once more
               break;
          }
       }
       
   }

You could also use a normal for-loop, but I would suggest using a list iterator if it is a linkedlist

int size = studentList.size();
for (int i=0; i<size; i++) {
    if (studentList.get(i).getStudentID().equals(id)) {
        studentList.remove(i);
        break;
    }
}

[pbl]

Does your Student class has an equals() method that identifies 2 Student to be equals() when their ID is equal ?
The LinkedList.remove(object) method will scan the LinkedList and remoce the first element in the list.equals(object)

Another alternative is to use the old for() loop way and to remvove by index

for(int i = 0; i < studentList.size(); ++i) {
  Student x = studentList.get(i);
  if(x.getId() == studentId) {
     studentList.remove(i);
     break;
  }
}

Actually, if using the equals() method, if you remove by ID no need to do the loop you can simply

void remove(int studendId) {
   Student s = new Student(studentId);
   list.remove(s);
}

*Edited: and as CasiOo mentionned your loop is really inneficient
If you the Student to remove is number 156,000 in the list, you will be scanning 1456,00 elements and then your call to remove(Student) will also have to scan those 156,000 element repeating your comparaison
Doing remove(156000) at least avoid to redo the comparaisons and will start by the end if the list has less than 312,000 elements

This post has been edited by pbl: 18 March 2013 - 12:30 PM

[parry2411]

When i run this code:

import java.util.LinkedList;

public class Registry
{
    public LinkedList<Student> studentList = new LinkedList<>();
    
    public int listSize = studentList.size();

    public Registry(){}
    
    
    public void addStudent(Student aStudent)
    {
        studentList.add(aStudent);
    }
    
    
    public void deleteStudent(int studentID)
    {
        for (int i = 0; i < listSize; i++)
        {
            Student x = studentList.get(i);
            if (x.getStudentID() == studentID)
            {
                studentList.remove(i);
                break;
            } 
        }
    }


           
    public static void main(String[] args)
    {
        Registry rg = new Registry();
        
         Student st1 = new Student("John", "Hughes", 010001);
         Student st2 = new Student("Ceri", "Hughes", 010002);
         Student st3 = new Student("Bob", "Parry", 010003);
         
         rg.addStudent(st1);
         rg.addStudent(st2);
         rg.addStudent(st3);
         
         System.out.println(rg.studentList.size());
         
         rg.deleteStudent(010003);
         
         System.out.println(rg.studentList.size());
                
    }
 

}

The output of list size before delete is 3 and after delete is 3, Even when i tosting the list i get the same results twice, Which means they dont delete

This post has been edited by parry2411: 18 March 2013 - 01:31 PM

[parry2411]

How would i go abouts on doing this using The Iterator?

[pbl]

Have you read CasiOo's post and mine ?

Post your Student class
Does it have

    public boolean equals(Object o) {
       Student other = (Student) o;
       return ID == other.ID;
    }

Iterator or not you'll end up with the same result.


And we both gave a way to do the stuff correctyl and more efficiently

This post has been edited by pbl: 18 March 2013 - 01:52 PM

[parry2411]

No it hasnt got this method there.

I dont need the items to be deleted if they are the same, basically the end product is a Registartion class where you can add, delete and update students that are stores in the linkedlist.

Now when you go to delete the interface will ask for which student do you want to delete, then they enter the students id, and then i call the deletestudent method to delete that student from the list.

[pbl]

So... the remove(Student method) will pass through all the Student in the link list and compare them

If you do not have a equals() method none of tehm will be equal() so your code fails
Here is the code of remove(object) from LinkedList.java

/**
     * Removes the first occurrence of the specified element from this list,
     * if it is present.  If this list does not contain the element, it is
     * unchanged.  More formally, removes the element with the lowest index
     * <tt>i</tt> such that
     * <tt>(o==null&nbsp;?&nbsp;get(i)==null&nbsp;:&nbsp;o.equals(get(i)))</tt>
     * (if such an element exists).  Returns <tt>true</tt> if this list
     * contained the specified element (or equivalently, if this list
     * changed as a result of the call).
     *
     * @param o element to be removed from this list, if present
     * @return <tt>true</tt> if this list contained the specified element
     */
    public boolean remove(Object o) {
        if (o==null) {
            for (Entry<E> e = header.next; e != header; e = e.next) {
                if (e.element==null) {
                    remove(e);
                    return true;
                }
            }
        } else {
            for (Entry<E> e = header.next; e != header; e = e.next) {
                if (o.equals(e.element)) {
                    remove(e); // this is another remove() that works by node
                    return true;
                }
            }
        }
        return false;
    }

See the line 24
if (o.equals(e.element)) {
so if your Student class does not have a method equals() that returns if 2 Students are equals or not (in your case have the same ID) it will never find two Student equals

Also, that method is not good as we have both written... a lot more efficient to remove() by index for the reasons explained

This post has been edited by pbl: 18 March 2013 - 02:40 PM
Reason for edit:: Added line number

[parry2411]

No student will have the same ID though, I create the Student object which are: Student st1 = new Student("Bob","Williams",678);

The ID of this student is 678. Now i have a class called Register which has a LinkedList to Hold These Sudent objects that i craete, Example no i have 10 students:

st1 st2 .................. st10.

On my register Class i have a method to ADD a student to the LinkedList and a method to Delete a Student. To delete a student you must pass in the students IDNumber wich in my case is 678 for example. Now if i call the deleteStudent method with a parameter of 678.

This now will look through the linkedlist for an element that has an id of 678.

When this element is found the object will be removed from the list. So st1 BOB WILLIAMS 678 will be removed from the List.

STUDENT CLASS

public class Student 
{
    private String foreName;
    private String surName;
    private int studentID;

    
    public Student(String foreName, String surName, int studentID)
    {
        this.foreName = foreName;
        this.surName = surName;
        this.studentID = studentID;
    }

    public String getForeName()
    {
        return foreName;
    }

    public void setForeName(String foreName)
    {
        this.foreName = foreName;
    }

    public String getSurName()
    {
        return surName;
    }

    public void setSurName(String surName)
    {
        this.surName = surName;
    }

    public int getStudentID()
    {
        return studentID;
    }

    public void setStudentID(int studentID)
    {
        this.studentID = studentID;
    }

    @Override
    public String toString()
    {
        return getClass().getSimpleName()+"{" + "foreName=" + foreName + ", surName=" + surName + ", studentID=" + studentID + '}';
    }
    
    public String format()
    {
        return String.format("%-15s \t %-15s \t %-15d \n",foreName,surName,studentID);
    }
}

Register Class

ackage student;

import java.util.LinkedList;

public class Registry
{
    public LinkedList<Student> studentList = new LinkedList<>();
    public int listSize = studentList.size();

    public Registry(){}
    
    
    public void addStudent(Student aStudent)
    {
        studentList.addLast(aStudent);
    }

    public void deleteStudent(int studentID)
    {
        for (int i = 0; i < listSize; i++)
        {
            Student x = studentList.get(i);
            if (x.getStudentID() == studentID)
            {
                studentList.remove(i);
                
            }
        }
    }

    public static void main(String[] args)
    {
        Registry rg = new Registry();

        Student st1 = new Student("John", "Hughes", 6);
        Student st2 = new Student("Ceri", "Hughes", 2);
        Student st3 = new Student("Bob", "Parry", 3);

        rg.addStudent(st1);
        rg.addStudent(st2);
        rg.addStudent(st3);


        for (Student element : rg.studentList)
        {
            System.out.println(element + "\n");
        }


       rg.deleteStudent(2);

        System.out.println("\nAFTERDELETE\n");
        
        for (Student element : rg.studentList)
        {
            System.out.println(element + "\n");
        }

    }
}

[pbl]

I don't know in which language I can tell you

The LinkList remove(Object) method when asked to remove and object scan the list, use the equals() method to identify the object to remove

   Student s1 = new Student("John", "Lennon", 101);
   Student s2 = new Student("John", "Lennon", 101);

   // this is internally what the remove() method will do
   System.out.println("Is s1 equals s2:", s1.equals(s2));

should print "false"
s1 and s2 are not equal until you write an equals() method that return true if they have the same ID

[pbl]

And that one does not work because listSize == 0

import java.util.LinkedList;

public class Registry
{
    public LinkedList<Student> studentList = new LinkedList<Student>();
    
    // listSize set to 0 at creation time
    public int listSize = studentList.size();

    public Registry(){}
    
    
    public void addStudent(Student aStudent)
    {
        studentList.add(aStudent);
    }
    
    
    public void deleteStudent(int studentID)
    {
// get new listSize here
        listSzie = studentList.size();
        for (int i = 0; i < listSize; i++)
        {
            Student x = studentList.get(i);