0

I have an arraylist of object User which contains a user's username and password. I've created a updateUsername method to attempt to change a user's username, and in it I've used indexOf, but it always returns -1(it can't find said user in the arraylist).

updateUsername method:

public void updateUsername(User user, String username) {
    ArrayList<User> users = getAllUsers();
    int i = users.indexOf(user);
    user.setUsername(username);
    users.set(i,user);
    synToFile(users);
}

This method is called in a controller when a button is clicked:

public JFXListView<Label> lview2;

@FXML
void changeUsername(ActionEvent event) {
    String username = newUserField.getText();
    UserDAO theDAO = new UserDAO();
    Label lbl = lview2.getSelectionModel().getSelectedItem();
    //the items in the listview are of object label
    User u = theDAO.getUser(lbl.getText());
    theDAO.updateUsername(u,username);
    ObservableList<Label> userList = theDAO.storeUsers();
    lview2.setItems(userList);
}

lview2 is a listview in a separate controller - I've instantiated here in the separate controller:

changeUsernameController cu = (changeUsernameController)fxmlLoader.getController();
cu.lview2 = listView;

Don't think these are necessary, but I've added the getAllUsers(), synToFile() and getUser() methods here as well:

public ArrayList<User> getAllUsers() {
    Scanner sc;
    String record = null;
    String[] fields;
    ArrayList<User> users = new ArrayList<User>();

    try {
        sc = new Scanner(dataFile);
        while (sc.hasNextLine()) {
            record = sc.nextLine();
            fields = record.split(";");
            String username = fields[0];
            String password = fields[1];
            User u = new User();
            u.setPassword(password);
            u.setUsername(username);
            users.add(u);
        }
    } catch (FileNotFoundException e) {
        System.out.println("No record found!");
        //e.printStackTrace();
    }
    return users;
}

public void synToFile(ArrayList<User> userList) {
    if (userList == null) {
        return;
    }

    try {
        FileWriter out = new FileWriter(dataFile);
        for (User u: userList) {
            out.append(u.toString() + "\n");
        }
        out.close();
    }catch (IOException e) {
        e.printStackTrace();
    }
}

public User getUser(String username) {
    ArrayList<User> users = getAllUsers();
    User user = null;
    for (User u: users) {
        if (u.getUsername().equals(username)) {
            user = u;
            break;
        }
    }
    return user;
}

Note: I added debug lines in the updateUsername() method - the ArrayList is as it should be, and the user object is correct as well.

User class: 

package Server;

import java.util.ArrayList;


public class User {

private String username;
private String password;
private ArrayList<Double> scoreList=new ArrayList<Double>();

public User() {

}


public String getUsername() {
    return username;
}

public void setUsername(String username) {
    this.username = username;
}

public String getPassword() {
    return password;
}

public void setPassword(String password) {
    this.password = password;
}

public String toString() {
    return username + ";" + password;
}

public String usernameString() {
    return username;
}
}
cosmo
  • 751
  • 2
  • 14
  • 42
  • Show the User class, please. Do you understand how indexOf finds an index? – OneCricketeer Jan 22 '17 at 14:23
  • 1
    Did you override `equals` in the `User` class? – Eran Jan 22 '17 at 14:24
  • `indexOf` uses `equals(Object)`, can you show us the code for that? – Peter Lawrey Jan 22 '17 at 14:24
  • If you don't override `equals`, two objects are only `equals` if they are the same object. – Peter Lawrey Jan 22 '17 at 14:27
  • 1
    The JavaDoc of `indexOf` is public, so you should read that to understand how it works and what **you** need to do to use it correctly. And when you don't know how to implement `equals`, when why don't you do research about that? You're not the first one asking this on SO. – Tom Jan 22 '17 at 14:27

5 Answers5

2

You should override the equals method in the User class. The indexOf method internally uses equals to see if each element in the array is equal to the input parameter.

See this answer on how to override equals in Java.

Community
  • 1
  • 1
Chetan Kinger
  • 15,069
  • 6
  • 45
  • 82
1

Here's the working version of the User class:

public class User {

private String username;
private String password;
private ArrayList<Double> scoreList=new ArrayList<Double>();

public User() {

}


public String getUsername() {
    return username;
}

public void setUsername(String username) {
    this.username = username;
}

public String getPassword() {
    return password;
}

public void setPassword(String password) {
    this.password = password;
}

public String toString() {
    return username + ";" + password;
}

public String usernameString() {
    return username;
}
@Override
    public boolean equals(Object obj) {
        if (obj instanceof User){
            User tmp = (User)obj;
            return tmp.getUsername().equals(getUsername());
        }
        return false;
    }

}

I added only the test on username in the equals() method, but you can also add other tests, based on your requirements.

Lino
  • 5,084
  • 3
  • 21
  • 39
0

IndexOf method uses equals to verify the presence or not of an object in the list...

so your USER class must properly override that method (hascode too)

that is the reason why the returned index is -1 even though the user object is in the list..

here the implementation

229     public int indexOf(Object o) {
230         if (o == null) {
231             for (int i = 0; i < size; i++)
232                 if (elementData[i]==null)
233                     return i;
234         } else {
235             for (int i = 0; i < size; i++)
236                 if (o.equals(elementData[i]))
237                     return i;
238         }
239         return -1;
240     }

refernce

ΦXocę 웃 Пepeúpa ツ
  • 47,427
  • 17
  • 69
  • 97
0

Unless you override equals two object are only equal if they are the same object. In you case you are assuming they have the same username. A better approach is to use a Map instead of list and use a String as the Key, though this make the User class redundant.

// Username to password
public Map<String, String> getAllUsers() {
    try (Stream stream = Files.lines(datafilePath)) {
       return stream
              .map(line -> line.split(";"))
              .collect(Collectors.toMap(l -> l[0], l -> l[1]));
}

// to update the user
Map<String, String> userToPasswordMap = getAllUsers();
String password = userToPasswordMap.remove(oldUsername);
if (password == null) // error
userToPasswordMap.put(newUsername, password);
syncToFile(userToPasswordMap);
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
-1

You need to Override the equals in your user class for it to work inside indexOf() method (as stated in the comments).

A Example would be.

        public class User {
        String username;
        String password;

        @Override
        public boolean equals(Object obj) {
            if (obj == null) {
                return false;
            }
            if (!User.class.isAssignableFrom(obj.getClass())) {
                return false;
            }

            final User other = (User) obj;

            if ((this.username == null) ? (other.username != null) : !this.username.equals(other.username)) {
                return false;
            }
            if (!this.password.equals(other.password)) {
                return false;
            }
            return true;
        }
    }

As said in the comment, IndexOf() doesn't use hashcode, I know that isn't related to the question, but it is convention to override both methods (equals and hashcode) because you are gonna have problems with other classes like HashMap or HashSet if you don't override it.

LouizFC
  • 161
  • 1
  • 10