Поиск конкретного объекта в ArrayList


Это эффективный способ поиска для определенного объекта в ArrayList?

Продукт является суперклассом. Мобо, процессора, оперативной памяти, GPU... являются подклассами. Коллекции(так называемый Сток) хранит объекты продукта. Я считаю, что они главные, я поиск нашел, когда класса и атрибута "модель"(вы можете увидеть добытчик атрибут в мой код) совпадают.

public static boolean checkAvailability(ArrayList<Product> stock, Product product) {
    boolean found = false;
    for(Product p: stock) {
        if(p instanceof Mobo && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof CPU && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof GPU && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof RAM && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof Monitor && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof Keyboard && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof Mouse && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
        if(p instanceof Printer && (product.getModel() == p.getModel())) {
            found = true;
            break;
        };
    };
    return found;
}


Комментарии
4 ответа

Ваш код очень влажный (в противоположность сухой). Он повторяет много. Начнем с того, что вы пытаетесь достичь:


  1. У вас есть ArrayList из ProductС.

  2. Вы хотите checkAvailability возвращает true, если любой Product в данной ArrayList имеет ту же модель и тип данного Product.

Как вы реализовали это, с instanceof проверяет, является ММО не подходит. Вместо этого, давайте изменить Product класса на заказ equals() метод сравнения равенства Productы, мы можем сохранить равенство логика внутри Product класс так это многоразовые.

В equals() метод на объект, по умолчанию, будет действовать аналогично == - то есть, он проверяет равенство ссылок, но это обычно используется, чтобы переопределить этот метод, чтобы сравнить логическое равенство вместо.

class Product {
...

@Override()
public boolean equals(Object other) {
// This is unavoidable, since equals() must accept an Object and not something more derived
if (other instanceof Product) {
// Note that I use equals() here too, otherwise, again, we will check for referential equality.
// Using equals() here allows the Model class to implement it's own version of equality, rather than
// us always checking for referential equality.
Product otherProduct = (Product) other;
return otherProduct.getModel().equals(this.getModel());
}

return false;
}
}

Конечно, это не потерять instanceof проверка в вашем checkAvailability способ, но вы можете легко восстановить нечто подобное путем переопределения equals() в подклассах Product если вы хотели восстановить этот чек (Спойлер: Вы, вероятно, не! Не сравнивай типы, если вам нужно). По крайней мере, попытаться перенести равенства логика в equals() метод на классе, а не сравнивая равенства вне класса. Инкапсуляция и повторное использование-это всегда приятно!

При этом методе, мы можем упростить checkAvailability следующему:

public static boolean checkAvailability(ArrayList<Product> stock, Product product) {
for (Product p : stock) {
if (p.equals(product)) {
return true;
}
}

return false;
}

Который может быть упрощен для использования Java-потоков, если вы хотите.

public static boolean checkAvailability(ArrayList<Product> stock, Product product) {
return stock.stream()
.anyMatch(p -> p.equals(product));
}

Или, еще лучше, мы можем сделать необходимый тип менее конкретными:

public static boolean checkAvailability(Collection<Product> stock, Product product) {
return stock.stream()
.anyMatch(p -> p.equals(product));
}

В качестве отступления: это обычно хорошая практика, чтобы переопределить hashCode() если вы переопределяете equals(). Не имея больше знаний о вашем решении, это для меня, чтобы сказать вам, как это делать сложно, но я бы порекомендовал вам сделать это, если вы в конечном итоге с помощью моего предложения.

Другая сторона Примечание: Если вы окажетесь делать много осознаний, я рекомендую checkAvailability принять Stream<Product> вместо Collection<Product> таким образом, чтобы избежать создания нескольких потоков.

12
ответ дан 11 апреля 2018 в 11:04 Источник Поделиться


  1. есть избыточная точка с запятой ';' в конце каждого если и в конце цикла (в основном после закрывающей фигурной скобки)

  2. код интерфейс: метод должен принимать аргумент List тип. этак абоненты могут выбрать список, который реализации использовать (например, LinkedList)

  3. булевская переменная является избыточным. вы можете сразу же вернуться true Если соответствие не найдено, в противном случае возврат false

        if(p instanceof Mobo && (product.getModel() == p.getModel())) {
    return true;
    }
    ...
    } // closing the for loop
    return false;

  4. последовательность, если вопросы могут легко быть объединены в одно условие:

    if ((p instanceof Mobo || p instanceof CPU || ...) && product.getModel() == p.getModel()) {
    return true;
    }

  5. но вместо допроса в отношении отдельных типов, вы можете поместить их все в коллекцию и запросов по локализации:

    private static Set<Class<? extends Product>> productTypes;  // class variable
    static { // static constructor
    productTypes.add(Mobo.class);
    productTypes.add(CPU.class);
    }

    // inside the method
    if (productTypes.contains(p.getClass()) && product.getModel() == p.getModel()) {
    return true;
    }


8
ответ дан 11 апреля 2018 в 11:04 Источник Поделиться

В instanceof ключевое слово, как правило, признак дурного наследства (по принцип замещения Лисков) и я не рекомендую использовать.

Кроме того, я не понимаю, что структура данных биржевой информации.

Почему не что-то вроде этого?

boolean stockAvailable(Map<ProductId, Integer> stock, Product product) {
if(!stock.contains(product.getProductId()) { // ... guard
return false;
}

return stock.get(product.getProductId()) > 0;
}

2
ответ дан 11 апреля 2018 в 03:04 Источник Поделиться

Вы проигнорировали мой комментарий с просьбой о разъяснении, поэтому я только предполагаю, что я был прав, думая, что ваш код на самом деле не делать то, что вы намерены это сделать. Вы пишете, что на матч, класс и модель должна совпадать, но ваш код сравнивает только модель на клавишу поиска, что товаров в наличии. Она не проверяет, является ли элемент из запаса имеет тот же класс в качестве ключа поиска, он просто констатирует, что класс, к которому относится продукт является одним из нескольких классов, независимо от того, этот класс также класс ключ поиска.

В самом деле, ваше словесное описание код очень простой, и перевод в коде не должно быть намного сложнее:

public static boolean checkAvailability(List<Product> stock, Product product) {
for(Product p: stock) {
if (product.getModel() == p.getModel()
&& product.getClass() == p.getClass()) {
return true;
}
}
return false;
}

Вы также можете вместо этого использовать поток:

public static boolean checkAvailability(List<Product> stock, Product product) {
return stock.stream().anyMatch((Product p) ->
product.getModel() == p.getModel()
&& product.getClass() == p.getClass());
}

Обратите внимание, что, в отличие от instanceofэто только проверяет классы на равенство и возвращает false если один класс является подклассом другого. Если вы хотите рассмотреть подклассы также, вы можете использовать Class.isAssignableFrom(Class).

Вы также должны знать, что, если Product.getModel() возвращает примитивный тип, вы сравниваете модели, основанные на равенство ссылок, а не сравнивая их содержание. Я не знаю какой тип Product.getModel() возвращается, но даже если это только возвращает простой String, == не будем рассматривать содержимое строки, но только проверить, действительно ли две переменные ссылаются на один объект (который может быть не обязательно даже в том случае, если строковое содержимое идентичны), так что вы должны использовать equals(Object) если вы хотите сравнить содержимое двух строк.

2
ответ дан 12 апреля 2018 в 08:04 Источник Поделиться