tags:

views:

111

answers:

5

Hi there! I'm new to java so im having some "annoying" problems. I have a class Employee which contains a int idNumber and a int phonenumber. Then i have a LinkedList 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){
                    employeeTemp.setNewPhoneNumber(newphone);
                    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)

+1  A: 
  • 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.
Michael Borgwardt
A: 

http://java.sun.com/j2se/1.4.2/docs/api/java/util/LinkedList.html

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)

Kurisu
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
HashMap or TreeMap are much better than a ArrayList for random access.
Milhous
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
A: 

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

getIdNumber();

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
Tom
+2  A: 

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) {
                    employee.setNewPhoneNumber(newphone);
                    return; 
                }
}

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.

Carl Manaster
I think that someone should clarify (ok, i´ll do it) that foreach constructs are syntactic sugar, and Iterators are used behind.
Tom
Yeah, iterators are fine as long as I don't have to look at 'em. :-)
Carl Manaster
you can also use break; rather then return if you're doing this in the middle of a function
Chad Okere
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
A: 

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)
}
missed the getter
Tom