Take the 2-minute tour ×

I'm new to java so im having some "annoying" problems. I have a class Employee which contains an int idNumber and a int phone number. Then I have a LinkedList<Employee> sorted by idNumber. I want to change the phonenumber of a certain idnumber. I've been working with Iterators but i don't know if i'm doing it right, which I doubt.

public void setNewPhoneNumber(int idnumber, int newphone){
        Iterator<IndexC> it = listEmployee.iterator();   
        IndexC employeeTemp = null;

        boolean found = false;
        while(it.hasNext() && !found){ 
                employeeTemp = it.next();
                if(employee.getIdNumber()== idnumber){
                    found = true; 

Yeah, I know employee.setNewPhoneNumber is wrong, but I don't know which the correct way change the value on the linkedlist. (Sorry for the bad english, not a native speaker)

share|improve this question
Doesn´t that work? What error do you get? Why do you think it wouldn´t work? –  Tom May 9 '09 at 0:47
add comment

5 Answers

Iterators are a pain; the foreach construct is a lot nicer:

public void setNewPhoneNumber(int idnumber, int newphone) {
        for (Employee employee : listEmployee)
                if (employee.getIdNumber() == idnumber) {

I'm not clear on what IndexC is, and I don't often use LinkedList - there could be some subtlety here I'm missing - but I think you're better off avoiding the iterators.

share|improve this answer
I think that someone should clarify (ok, i´ll do it) that foreach constructs are syntactic sugar, and Iterators are used behind. –  Tom May 9 '09 at 1:08
Yeah, iterators are fine as long as I don't have to look at 'em. :-) –  Carl Manaster May 9 '09 at 1:27
you can also use break; rather then return if you're doing this in the middle of a function –  Chad Okere May 9 '09 at 8:30
But I am a big fan of small routines that do only one thing. If I were using break, it would be because my routine was doing two (or more) things. –  Carl Manaster May 9 '09 at 18:48
add comment
  • You're not "changing parameters in a Linked List", you're trying to find an object in a list and change a property of that object
  • You should be using a Map (such as a HashMap) instead of a List, then you won't have to iterate.
  • If you iterate, use the for loop: for(IndexC employeeTemp: employeeTemp){}
  • Changing the phone numer would conventionally be done through a setPhoneNubmer() method, but it depends entirely on the IndexC class whether it has such a method. Look at that class's definition.
  • When asking a question, always include error messages! "It doesn't work" is a really useless piece of information.
share|improve this answer
add comment

my bad, IndexC is the Employee class, "bad copy past" sorry. I don't like LinkedList but i have to use it with +5000 entries (School Exercise). I don't think using for's with so many entries is recommended. The class as set's, get's, clones..

class Manager{
private LinkedList<Employee> listE = new LinkedList<Emploee>;

public void setNewPhoneNumber(int idnumber, int newphone)
share|improve this answer
missed the getter –  Tom May 9 '09 at 1:45
add comment

One reason for it not to work is that there´s no IndexC in the list that satisfies (employee.getIdNumber()== idnumber).

Maybe you should post some extra code, for example, where is that list created, have you filled it with anything?

Besides, what is it that doesn´t work? The setting of the new phone number, or retrieving the element from the list?

In both cases, i think that you should post both methods, that is


As Mike B. says, maybe using a Map implementation would be better. Since you are considering order, maybe a SortedMap (such as TreeMap) implementation could be better.

In any case, rember you have to override two methods in your IndexC (when using maps). Otherwise, things will get messy.

  • equals
  • hashCode
share|improve this answer
add comment


You want to use a for loop with an int incrementing till you find the object you want. Then you want to use listEmployee.get() to get the object you want and edit it.

However, if you need random access to items like that, then you should not be using Linkedlists. Stick it in an ArrayList instead. That has much better random access time.

As a side note, you don't even need the for loop if the id numbers are in order from 0-whatever. You can simply listEmployee.get(idNumber)

share|improve this answer
That listEmployee.get(idNumber) will only work if you have at least idNumber elements in it. It will get the idNumberth element of the list. It´s not a map. –  Tom May 9 '09 at 0:50
HashMap or TreeMap are much better than a ArrayList for random access. –  Milhous May 9 '09 at 1:40
If you want to look up a particular object by key, then you should be using a map, rather than a list. Also, when iterating over a list, prefer the foreach loop, or some other method which uses a list iterator, since the underlying implementation can more efficiently perform the iteration. –  Rob May 9 '09 at 1:41
add comment

Your Answer

