0

I have a linked list full of employees for testing, I would like to iterate through it and see if an employee is found then return true, this does work slightly however it will only find the last employee in the list, what am I doing wrong? Also I have different types of employee such as manager who inherit from the account class, sorry if thats confusing! Sorry still a beginner!! Thanks

Here is my list with the accounts added:

LinkedList<Account> Accounts = new LinkedList<Account>(); 

Employee John = new Account("John", "password1");
Manager Bob = new Account("Bob", "password2");

Here is the method which finds the employee/account.

private boolean check(String employee) {

                for (Account e : Accounts) {
               if (e.getEmployee().equals(employee)) {
                   return true;
               }
               }       
                return false;


    }

EDIT: Below is the code i have added, it works however only for the last user. I would like the method to return true if the account name is found in the linked list, so that the system can continue and ask for further info etc.., the name variable is located in my Account class

private boolean check(String employee){
     for(Account a : accounts){
        if(a.name.equals(employee)){
            return true;
        }
     }
     return false;
 }
aluckii
  • 39
  • 2
  • 9
  • It sounds like a problem with your traversal logic. You should post the code for that – Alex Mar 02 '15 at 20:16
  • What does getEmployee() return? If it's not a string you need to write an Equals method – JNYRanger Mar 02 '15 at 20:17
  • Does your Account class have equals() method? If it does not, Java will use default one which only checks if two objects are the same instance and will not match two instances with the same data. See [here](http://www.leepoint.net/data/expressions/22compareobjects.html) for more details. – Marko Živanović Mar 02 '15 at 20:18
  • @MarkoŽivanović Hi no my Account class does not have an equals method, I will try to implement this now – aluckii Mar 02 '15 at 20:26
  • 1
    This code posted could be shortened using Java 8 to `return Accounts.stream().anyMatch(employee::equals);` (by the way, use camelCase for variable names - `accounts`, not `Accounts`). – bcsb1001 Mar 02 '15 at 20:43

4 Answers4

0

No need to iterate, list's contains method will do it for you

Tommaso Bertoni
  • 2,333
  • 1
  • 22
  • 22
  • This will only work if equals() is implemented properly (`@Override`) for the Account class. – JNYRanger Mar 02 '15 at 20:18
  • @JNY `equals()` could not be implemented 'properly' in this case, since any `equals()` method on the `Accounts` class that returned `true` when passed a `String` would violate the equals contract, specifically the part about it being reflexive. – David Conrad Mar 02 '15 at 20:24
  • So would I need to have an equals() in my Account class? – aluckii Mar 02 '15 at 20:27
  • @DavidConrad Nice catch, you are absolutely correct. This would not work for string comparison. – JNYRanger Mar 02 '15 at 20:30
  • @aluckii No, this answer would not work for you to do string comparison. – JNYRanger Mar 02 '15 at 20:30
  • 1
    @aluckii No, because contains() depends on the list's underlying class's equals() method as seen in the JavaDocs – JNYRanger Mar 02 '15 at 20:36
0
if (e.getEmployee().equals(employee))

This will compare if the objects are the same, meaning they are the same in memory.

You need to specify what properties of an object have to equal in order to logically say that 2 different objects are the same when looking at one or more properties.

For example

John.equals(Bob)

will never be true but if the object John and object Bob both have the name property a String = "Alex" then 

John.getName().equalsIgnoreCase(Bob.getName())

will be true

Code Whisperer
  • 1,041
  • 8
  • 16
  • So I would need a new method which does this, sorry but how would I start start? – aluckii Mar 02 '15 at 20:29
  • so would it be something similar to equals (employee) { if (employee == account) return true? , just an example of the variable names – aluckii Mar 02 '15 at 20:32
  • 1
    Also OP should make sure that he is comparing "apples-to-apples" since currently it shows comparing an Employee object to a String object. – JNYRanger Mar 02 '15 at 20:34
  • @JNYRanger ah okay so I should be comapring these to actual Accounts? rather than strings? – aluckii Mar 02 '15 at 20:36
  • You need to compare the properties of an object to the properties of another object in order to deduce logically if they are equal. Bluntly comparing two objects will just compare their memory address. Java does not know how you defined your object and property fields you gave it when you are using equals() so you have to go deeper and pick specific fields you want to compare between two objects. You need to provide your own compare method and make it part of the object so you can easily call something like `Bob.customCompare(John);` – Code Whisperer Mar 02 '15 at 20:36
  • @CodeWhisperer Hi thanks, I think understand what you are saying, but i am unsure how to start, could you possibly point me in the right direction? thanks – aluckii Mar 02 '15 at 20:41
  • @CodeWhisperer ah okay I will try now – aluckii Mar 02 '15 at 20:41
  • @aluckii some examples here: [Examples](http://stackoverflow.com/questions/16069106/how-to-compare-two-java-objects) – Code Whisperer Mar 02 '15 at 20:42
0

There is no need to iterate over the list. So, you can replace your check method with 

  private boolean check(String employee) {
           return Accounts.contains(employee);
}

You should further replace

 Employee John = new Accounts("John", "password1");
 Manager Bob = new Accounts("Bob", "password2");

with

 Employee John = new Account("John", "password1");
 Manager Bob = new Account("Bob", "password2");
René Winkler
  • 6,508
  • 7
  • 42
  • 69
0

You have two options here. The first is to change the signature of your check method to use an Account object, or extract a field from your Account object for comparison as such:

private boolean check(Account account){
      return accounts.contains(account);
}

However, this would require you to implement a custom equals() method for Account:

@Override
public boolean equals(Object obj){
    if(obj instanceof Account){
         Account acct = (Account)obj;
         //Compare any internal fields that mean it's "equal"
         if(acct.prop1 == this.prop1 && ...) {
            return true;
          }
    }

    return false;
}

Your other option is to just compare against a particular field in your iteration:

private boolean check(String emp){
     for(Account a : accounts){
        if(a.stringProperty.equals(emp)){
            return true;
        }
     }
     return false;
 }
JNYRanger
  • 6,829
  • 12
  • 53
  • 81
  • hi thanks so if i was to create my own equals() method would i simply change in the check method - return accounts.contains(account); to return accounts.equals(account); – aluckii Mar 02 '15 at 21:09
  • Also in your second solution, could i replace the 'stringProperty' with say the username of that account? thanks – aluckii Mar 02 '15 at 21:10
  • In order for the first method to work, you need to implement an equals method within your Account class, otherwise it'll just check if that exact same object exists in the collection. – JNYRanger Mar 02 '15 at 21:11
  • Yes of course, I just used a generic name as an example. Just make sure you are comparing the same types. – JNYRanger Mar 02 '15 at 21:11
  • Hi thank you I have tried your second solution it does work for my last user, I'm not sure why this is happening as it has happened before :( – aluckii Mar 02 '15 at 21:23
  • actually wait if the variable of say username is protected in Account, would that be a problem? – aluckii Mar 02 '15 at 21:25
  • You should set a public getter for that and use that in the comparison if you're using the second method. If you're using a public equals method then it doesn't matter because the comparison is happening within the object instance – JNYRanger Mar 02 '15 at 21:30
  • Also, what do you mean by "last user" should there be another match happening earlier? – JNYRanger Mar 02 '15 at 21:31
  • Sorry I mean I have a list of accounts and when say comparing the username, only the last account is being returned true – aluckii Mar 02 '15 at 21:43
  • Why would that be an issue? Are there other accounts that should have matched the criteria earlier in the collection? – JNYRanger Mar 02 '15 at 21:50
  • Well yes if I'm comparing for one account and check returns true and then I search to check for another and if returns false, they are many accounts in the linked list but the check() method is only returning true for my last value in it – aluckii Mar 02 '15 at 21:58
  • Edit your question and post your current code because it sounds like we're missing something otherwise it sounds like your comparison is only matching on that last item in your list and you need to use a different comparison. – JNYRanger Mar 02 '15 at 21:59
  • How are you sure that it's only your last element in the list that's returning true? Are you tracing/debugging this? It returns true as soon as it finds one match. – JNYRanger Mar 03 '15 at 00:33
  • Also (although this doesn't have to do with the question at hand) you should be using public getters with private fields, such as getName(). Avoid directly accessing public fields – JNYRanger Mar 03 '15 at 00:35
  • Yes i'm sure it is, I have added a few more employees and managers to the Accounts list and for some reason only the last element is being accepted as true, I probably have a tiny error somehwere which i will try and find – aluckii Mar 03 '15 at 08:41
  • okay i'll use public getters instead, that makes more sense, than using private directly ha!, Thanks :) – aluckii Mar 03 '15 at 08:42