Цикл возвращает истину, даже если должен вернуть ложь - PullRequest
1 голос
/ 06 декабря 2011

Я делаю массив чисел, соответственно

private ArrayList<Integer> numbers = new ArrayList();

, и я должен проверить, все ли они уникальны.Итак, у меня есть этот код:

public boolean isUnique()
{
    ArrayList<Integer> checkNumbers = new ArrayList();

    for(int i = 1; i<=numbers.size(); i++)
    {
        if(numbers.contains(i) && !checkNumbers.contains(i))
        {
            checkNumbers.add(i);
            return true;
        }           
    }

    return false;
}

Идея заключается в том, что я должен взять квадратное число (n) целочисленных входных данных, уникальное из 1 to n.

Но независимо от того,Я добавляю к числам (13 2 13 2), он всегда возвращает true.

Что здесь с моей логикой?

Ответы [ 8 ]

2 голосов
/ 06 декабря 2011

, если список может содержать больше чисел, чем n, и все, что вам нужно, это проверить, что 1 .. n все существуют и без дублирования, тогда ваш код должен быть изменен на это:

public boolean isUnique()
{
    ArrayList<Integer> checkNumbers = new ArrayList();

    for(int i = 1; i<=numbers.size(); i++)
    {
        if(numbers.contains(i))
        {
           if (!checkNumbers.contains(i)) 
             checkNumbers.add(i);
           else 
            return false; 
        }
        else{
            return false;
        }           
    }

    return true;
}

если, с другой стороны, список не может содержать более n элементов, другой список вам вообще не нужен:

public boolean isUnique()
    {

     if (numbers.size()<n)
        return false;  

        for(int i = 1; i<=numbers.size(); i++)
        {
            if(!numbers.contains(i))
              return false; 

        }

        return true;
    } 
2 голосов
/ 06 декабря 2011

Вам нужно перебрать содержимое numbers, а не их индексы.

0 голосов
/ 06 декабря 2011

Проблема в вашем методе заключается в том, что вы проверяете, находится ли каждое значение i в ArrayList.Что вам нужно сделать, это использовать get(), поэтому вы должны сделать это:

public boolean isUnique()
{
    ArrayList<Integer> checkNumbers = new ArrayList();

    for(int i = 1; i<=numbers.size(); i++)
    {
        if(numbers.contains(checkNumbers.get(i)) && !checkNumbers.contains(numbers.get(i)))
        {
            checkNumbers.add(i);
            return true;
        }           
    }

    return false;
}
0 голосов
/ 06 декабря 2011

Проблема в том, что вы сравниваете индекс вместо значения индекса:).

0 голосов
/ 06 декабря 2011

Вы создаете новый массив;

ArrayList<Integer> checkNumbers = new ArrayList();

и затем в цикле вы делаете это;

if (numbers.contains(i) && !checkNumbers.contains(i))
{
   checkNumbers.add(i);
   return true;
} 

!checkNumbers.contains(i) будет всегда будет правдой; Вы только что создали это; оно пустое. Поэтому в первый раз numbers.contains(i) будет true, метод вернет true.

0 голосов
/ 06 декабря 2011

Причина, по которой ваш метод возвращает true, состоит в том, что вы создаете checkNumbers внутри него, поэтому он пуст. Неважно, что вы указали в числах - оно не будет найдено в checkNumbers, добавьте его и верните с true.

Попробуйте истолковать ваш checkNumbers с помощью номера.

0 голосов
/ 06 декабря 2011

checkNumbers всегда пусто для начала, поэтому, когда он в первый раз находит значение i в вашем списке, он добавит это число к checkNumbers, а затем return true.

Я бы изменил логику - в первый раз, когда он находит число, которое находится в списке И в checkNumbers, возвращает false.Если этого никогда не произойдет, верните true.

0 голосов
/ 06 декабря 2011

Вы проверяете, содержит ли список значение, равное индексам, а не числам в списке. Вам нужно get() значение в индексе (или использовать цикл foreach).

У вас также есть оператор возврата сразу после добавления числа к checkNumbers, поэтому список выйдет из области действия сразу после добавления первого числа. Поэтому он никогда не будет содержать число при выполнении оценки if.

...