Array list not being searched properly

Question

I am having a hard time to get this to work. The isLoop() method gets called every time something is added to the array to make sure there are no duplicates. When there is a duplicate, it should return true. But it isn't.

isLoop(){
    if(!Arrays.asList(SQUARES).contains(sq)){
        return false;
    }else{
        return true;
    }
}

Am I missing something fundamental about arrays? I am fairly new to Java, so it is probably the case! :D

EDIT

I have been asked to show how the objects were instansiated, so I have posted the whole class here:

class Happy {
static int SQUARES[] = new int[10000];
static int sq;

public static boolean isHappy(int num){
    int s = 0;
    int n = num;

    int i=0;

    for(sq = num; (!isLoop()) && (sq > 1);){
        sq = addSquaresOfDigits(sq);
        SQUARES[i] = sq;
        i++;

    }

    if(isLoop()){
        return false;
    }else{
        return true;
    }
}

static int addSquaresOfDigits(int number) {
    int result = 0;
    int tmp;
    while(number > 0) {
        tmp = number % 10;
        result += tmp * tmp;
        number /= 10;
    }
return result;
}

static boolean isLoop(){
    if(!Arrays.asList(SQUARES).contains(sq)){
        return false;
    }else{
        return true;
    }
}

The method isHappy() will test the number given it to see whether it is a happy number or not.

What does SQUARES contain? Is it a type you've defined? It could be you have a buggy implementation of `equals()`. — Adrian Petrescu, Feb 16 '11 at 00:47
what is sq? what was originally added to the array? does sq's class have override the equals() method? — vanza, Feb 16 '11 at 00:47
sq is the current value of the item being added to the array. It does not override the equals() method — Isaac Lewis, Feb 16 '11 at 00:53

score 0 · Accepted Answer · edited May 23 '17 at 11:47

0

Make sure you define equals() and hashCode() on the sq class. See this question for details on that.

If the sq instance doesn't implement equals() then it will use the default implementation in Object which is to check if the two objects are the same instance.

edited May 23 '17 at 11:47

Community

1
1

answered Feb 16 '11 at 00:47

Valdis R

2,095
1
18
36

sq must be an instance of a class, or the snippet you've added wouldn't compile. Can you update the question to show how you've instantiated `sq`? – Valdis R Feb 16 '11 at 01:00

score 0 · Answer 2 · answered Feb 16 '11 at 00:56

0

For a start you can simplify your code to the following:

isLoop(){
    return Arrays.asList(SQUARES).contains(sq);
}

Despite what you've said elsewhere, SQUARES must be an array and sq must be an object instance of a Class or your code wouldn't compile.

The equals() method of sq will have to return true when called on one of the objects in SQUARES for isLoop() to return true.

answered Feb 16 '11 at 00:56

Alb

3,601
6
33
38

Thanks... I am new to Java, so the little details slip by me. – Isaac Lewis Feb 16 '11 at 01:20

score 0 · Answer 3 · answered Feb 16 '11 at 01:08

0

Consider switching to some Set implementation (e.g. HashSet class) to search if collections does or does not contain particular element. It's much faster then looping over the list. A

As mentioned aboce, remember about equals() and hashcode() methods.

answered Feb 16 '11 at 01:08

Krzysztof Wolny

10,576
4
34
46

score 0 · Answer 4 · answered Feb 16 '11 at 01:09

You state in a comment you haven't overriden equals() or hashCode(), this will be your issue as by default (and by design) Java doesn't know if two objects are meaningfully equal (since this will differ depending on the class!)

Have a look here for a good guide on the equals() and hashCode() methods and how to implement them properly. Alternatively, if you're feeling lazy, many IDEs will implement them for you based on a number of chosen fields. I'd only recommend this approach when you know how they work however!

As an aside, it seems you're using a list here when a HashSet would be a much better choice - by design it doesn't contain duplicates and is very fast at detecting them (it doesn't need to go through the entire list to check them as you're doing here.)

I just edited my original question to include the full class that the isLoop() method sits in. All other begginner java mistakes aside, how would I use a Hash Set? Like this? `Set SQUARES = new HashSet();` — Isaac Lewis, Feb 16 '11 at 01:19

score 0 · Answer 5 · answered Feb 16 '11 at 06:56

Why are you converting SQUARES to list , i think that to use contains method ,you are using this conversion.

For example consider ,if you want to get the index of an value i.e. static int sq = 9;

Arrays.binarySearch(SQUARES,sq);  // returns the index at which the value 9 is present

Array list not being searched properly

5 Answers5