Why is this if statement always returning true?

Question

Suppose: list = ["be", "be", "is", "not", "or", "question", "that", "the", "to", "to"]

public static void removeDuplicates(ArrayList<String> list) {
    for(int i = 0; i < list.size(); i++) {
        String s = list.get(i);
        for(int j = 0; j < list.size() - 1; j++) {
            String s2 = list.get(j);
            if(s2.equals(s)); {
                list.remove(j + 1);
            }
        }
    }
}

When I debug it, s2.equals(s) seems to be returning true all the time when s2 and s does not equal to each other at all. What am I doing wrong?

Just as an additional hint: You might consider using a Set instead of an ArrayList, so you do not have to deal with duplicates at all. — Seb, May 30 '14 at 08:21
See this one : http://stackoverflow.com/questions/203984/how-do-i-remove-repeated-elements-from-arraylist — Denys Séguret, May 30 '14 at 08:26

Axel · Accepted Answer · 2014-05-30T08:29:08.367

3

Remove the semicolon:

        if(s2.equals(s)); {

Should be

        if(s2.equals(s)) {

EDIT: Explanantion: In your code, you have an empty statement after the if, and so the block will be executed regardless of the comparison result:

public static void removeDuplicates(ArrayList<String> list) {
    for(int i = 0; i < list.size(); i++) {
        String s = list.get(i);
        for(int j = 0; j < list.size() - 1; j++) {
            String s2 = list.get(j);
            if(s2.equals(s))
                ; // <-- conditional
            {     // <-- unconditional
                list.remove(j + 1);
            }
        }
    }
}

edited May 30 '14 at 08:29

answered May 30 '14 at 08:19

Axel

13,939
5
50
79

I didnt realize I put semiconlon there... Thank you. Why is semicolon always returning the if statement true? Just curious. – Kibbles May 30 '14 at 08:26
It isn't. The semicolon just stands for an empty statement and terminates your then-clause. You might want to have a look at http://stackoverflow.com/questions/14112515/semicolon-at-end-of-if-statement – Seb May 30 '14 at 08:33

Denys Séguret · Answer 2 · 2014-05-30T08:29:44.190

1

You're testing elements against themselves too and you have a semicolon effectively removing the if. You probably want

if (i!=j && s2.equals(s)) {

edited May 30 '14 at 08:29

answered May 30 '14 at 08:17

Denys Séguret

372,613
87
782
758

Deepak Bhatia · Answer 3 · 2014-05-31T06:00:04.050

1

You should start your second loop from i+1, because in every iteration for j loop the i value will be equal to j and it will satisfy the equals condition ....

public static void removeDuplicates(ArrayList<String> list) 
{
  for(int i = 0; i < list.size(); i++) 
  {
    String s = list.get(i);
    for(int j = i+1; j < list.size(); j++) 
    {
        String s2 = list.get(j);
        if(s2.equals(s))             
        {   
           list.remove(j + 1);
           j--; //Subtract one as j+1 element is removed from list and index changes
        }

    }
  }
}

edited May 31 '14 at 06:00

answered May 30 '14 at 08:19

Deepak Bhatia

6,230
2
24
58

Also the semicolon needs to be removed. – Peter Niederwieser May 30 '14 at 08:21
what peter said. Also, list.remove should remove `j`, not `j+1` – Aserre May 30 '14 at 08:24
@Ploutox if `j` element is removed then the loop should break, because if the same element is occuring furthur in the array then the next element which will be removed might not be duplicate or the wrong element will be removed.... – Deepak Bhatia May 31 '14 at 06:01

score 1 · Answer 4 · answered May 30 '14 at 08:28

1

You have an extra semicolon. Do as follows:

if(s2.equals(s))             
    list.remove(j + 1);

and this change should work.

answered May 30 '14 at 08:28

Shrikant Kakani

1,511
2
17
37

Why is this if statement always returning true?

4 Answers4