Adding unique objects to ArrayList

Question

I'm trying to create a basic mobile phone application. I'm trying (but failing) to implement a feature to query existing contacts on the phone. I have done this by creating a method which returns an ArrayList object containing contacts. This is supposed to work by searching (using a for loop) to see if a particular contact exists, and adding it to the ArrayList if so. Thereafter, the method should return an ArrayList object containing nothing but the results of the query. However, when testing my code, I find that EVERY contact is added to the ArrayList regardless of whether it matches the query or not. Here are some snippets of my code:

Phone.java

private ArrayList<Contact> contacts;

public Phone() {
    this.contacts = new ArrayList<>();
}

public ArrayList<Contact> queryContacts(String contactName) {
       ArrayList<Contact> contactsList = new ArrayList<>();

        for (Contact contact : this.contacts) {
            if (this.findContact(contactName)) 
                contactsList.add(contact);
        }
        return contactsList;
    }

    private boolean findContact(String contactName) {
            for (Contact contact : this.contacts) {
                if (contact.getName().equals(contactName))
                    return true;
            }
            return false;
}

Testing my code (Main.java)

public static void main(String[] args) {
        char c= 'A';

        for (int i = 0; i < 10; i++) {
            //Create contacts with unique data
            phone.addContact(Contact.createContact("Contact"+c++, "07"+i));
        }

        System.out.println(phone.queryContacts("VoidContact")); //Dubious entry
        System.out.println(phone.queryContacts("ContactB")); //This entry exists
    }

Rightfully so, I receive no output upon invoking the phone.queryContacts() method with the "VoidContact" parameter. However, upon invoking it with the legitimate parameter "ContactB", rather than just receiving just one contact, I get the following output (please note that I have overrided Object.toString() in my Contact class):

Name: 'ContactA' Number: 070
Name: 'ContactB' Number: 071
Name: 'ContactC' Number: 072
Name: 'ContactD' Number: 073
Name: 'ContactE' Number: 074
Name: 'ContactF' Number: 075
Name: 'ContactG' Number: 076
Name: 'ContactH' Number: 077
Name: 'ContactI' Number: 078
Name: 'ContactJ' Number: 079

My question; why is outputting every contact as opposed to the unique contacts based on the inputted parameters? Thanks much in advance for your respones.

Have you posted exactly the code that you are trying ? It seems not, since in what you posted, when doing contactsList.add(contact), contactsList has never been defined. Thus this shouldn't compile successfully — Vic Seedoubleyew
– Vic Seedoubleyew, Commented Apr 8, 2016 at 21:06
@VicSeedoubleyew Dunno how I missed that. I've added the missing code. Thanks — Mr Chasi
– Mr Chasi, Commented Apr 8, 2016 at 21:09

JB Nizet · Accepted Answer · 2016-04-08 21:06:58Z

2

First of all, your code doesn't compile: the contactsList variable is not defined anywhere.

Second, your logic is flawed:

    for (Contact contact : this.contacts) {
        if (this.findContact(contactName)) 
            contactsList.add(contact);
    }
    return contactsList;

Let's translate this in English: for each contact, if the list contains contactName, the contact is added to the list. So, if the list contains contact name, all the contacts are added to the list, otherwise, none is added.

What you actually want is a method that finds the contacts in the list which have the given name:

public List<Contact> queryContacts(String contactName) {
    List<Contact> contactsList = new ArrayList<>();
    for (Contact contact : this.contacts) {
        if (contact.equals(contactName)) { 
            contactsList.add(contact);
        }
    }
    return contactsList;
}

Or, with Java 8:

public List<Contact> queryContacts(String contactName) {
    return contacts.stream()
                   .filter(contact -> contact.getName().equals(contactName))
                   .collect(Collectors.toList());
}

answered Apr 8, 2016 at 21:06

JB Nizet

694k94 gold badges1.3k silver badges1.3k bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Mr Chasi Over a year ago

Thanks for the comment. However, though I now know it was wrong, I'm still failing to understand the flaw in the original logic?

JB Nizet Over a year ago

Imagine the contacts contain Alice, Bob, Carl and Dominic. You're looking for all the contacts named Carl. You start your loop with the contact Alice. And you call findContact(Carl). It returns true, since Carl exists in the list. So you add the current contact, Alice, to the list. Then the loop continues with Bob. Does Carl exists in the contacts. Yes, so let's add the current contact, Bob, to the list. Etc. etc.

Mr Chasi Over a year ago

Ahh, great explanation. Thanks very much, that's really helped to clear it up

Community · Accepted Answer · 2019-09-26 12:11:52Z

2

The best solution is use HashSet

Set<Contact> collection=new HashSet<Contact>();
//...
for (Contact contact : this.contacts) {
   collection.add(concat);
}

anyway, if you want use ArrayList:

for (Contact contact : this.contacts) {
   if(!collection.contains(contact){
      collection.add(concat);
   }
}

in both solutions you must override equals method in Contact.class

public boolean equals(Object o){
   if(o == null) return false;
   if(o instanceof Contact){
      Contact c=(Contact)o;
      return c.name.equals(this.name);
   }
   return false;
}

edited Sep 26, 2019 at 12:11

CommunityBot

11 silver badge

answered Apr 8, 2016 at 21:22

Victor1125

7125 silver badges19 bronze badges

Collectives™ on Stack Overflow

Adding unique objects to ArrayList

2 Answers 2

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related