Использование ArrayList.indexOf при поиске значений внутри объекта - PullRequest
Новая
       10

Использование ArrayList.indexOf при поиске значений внутри объекта

0 голосов
/ 28 апреля 2019

У меня есть класс филиала, в котором есть массив объектов клиентов.В классе филиала я хочу addTransaction для данного имени клиента, но сначала я хочу проверить, существует ли клиент, а затем добавить транзакцию.

private boolean customerExists(String customerName) {
    for (int i = 0; i < customers.size(); i++) {
        if(customers.get(i).getCustomerName().equalsIgnoreCase(customerName)) {
            return true;
        }
    }
    return false;
}


private int customerPosition(String customerName) {
    for (int i = 0; i < customers.size(); i++) {
        if(customers.get(i).getCustomerName().equalsIgnoreCase(customerName)) {
            return i;
        }
    }
    return -1;
}



public void addTransaction(String customerName, Double transactionValue) {
    if(customerExists(customerName) == false) {
        System.out.println("Customer with name " + customerName + " not found");
    } else {
        customers.get(customerPosition(customerName)).addTransaction(transactionValue);
    }
}

Я знаю, что этот код будет работать, однако я уверен, что мне нужно сделать 2 цикла по массиву, чтобы проверить его существование и получить его позицию.Что кажется неэффективным

Я знаю, что метод indexOf был бы полезен в addTransaction, но я не уверен, как его использовать, если я ищу конкретное значение внутри объекта, а не объектасам, то есть я не ищу объект Customer, я ищу значение внутри объекта.Буду признателен за любой совет.

Редактировать: Спасибо всем, эти ответы фантастические.

Ответы [ 6 ]

3 голосов
/ 28 апреля 2019

В общем, вы не можете сделать что-то лучше, чем просто пройтись по списку, чтобы найти значение.

С точки зрения дублирования кода, заменить тело customerExists на: 

return customerPosition() >= 0;

И, в addTransaction(), сохраните результат customerPosition() в переменной и используйте thatVariable >= 0 вместо вызова customerExists().

Если вы используете Java 8+, вы можете использовать потоки: 

Optional<Customer> cust = customers.stream().filter(c -> c.getCustomerName().equals(customerName)).findFirst();

Тогда вам вообще не нужно беспокоиться об использовании индекса.

1 голос
/ 28 апреля 2019

Вместо использования нескольких для цикла, используйте один для цикла и получите объект клиента и используйте его, 

private Customer getCustomerUsingName(String customerName) {
    for (int i = 0; i < customers.size(); i++) {
        if(customers.get(i).getCustomerName().equalsIgnoreCase(customerName)) {
            return customers.get(i);
        }
    }
    return null;
}

Используйте клиента в вашем методе 

public void addTransaction(String customerName, Double transactionValue) {
    Customer customer = getCustomerUsingName(customerName)

    if(customer == null) {
        return;
    }
    customer.addTransaction(transactionValue);
}
0 голосов
/ 28 апреля 2019

Согласен с первым ответом Энди Тернера. После findFirst() вы можете обратиться к ifPresent потребителю, который добавит транзакцию. 

customers.stream()
        .filter(c -> c.getCustomerName().equals(customerName))
        .findFirst()
        .ifPresent(customer -> customer.addTransaction(transactionValue));

Если в будущем вам потребуется добавить транзакцию для всех клиентов, которые соответствуют предикату фильтра, вы можете использовать forEach и передать потребителю из предыдущего примера. 

customers.stream()
        .filter(c -> c.getCustomerName().equals(customerName))
        .forEach(customer -> customer.addTransaction(transactionValue));

В случае, если вы используете версию Java ниже 8, ваши циклы могут быть немного улучшены. Вы можете иметь один цикл 

Iterator it = customers.iterator();
while (it.hasNext()) {
    Customer customer = it.next();
    if (customerPredicate.test(customer)) { // your if condition
        customer.addTransaction(transactionValue); // your consumer.
        break; // If you need to change only first customer.
    }
}

Техническая записка

Stream имеет методы forEach и peek . Если вам нужно применить своего потребителя ко всем элементам потока, и при этом у вас все еще есть новый поток (ленивая операция) - используйте peek. Если вы хотите применить своего потребителя ко всем элементам и завершить поток (вам больше не нужен поток) - используйте операцию терминала - forEach.

0 голосов
/ 28 апреля 2019

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

, как правило, ArrayList в некоторых случаях быстрее, чем HashTable, но когда вам приходится искать элемент, HashTable (используя ключ для поиска) работает быстрее, чемArrayList, вы можете использовать его: 

HashMap<String, Customer> myCustomers = new HashMap<String, Customer>();
myCustomers.put("NameID_1", customerObject_1);
myCustomers.put("NameID_2", customerObject_2);
myCustomers.put("NameID_3", customerObject_3);
myCustomers.put("NameID_4", customerObject_4);

System.out.println(myCustomers.get("NameID_1"));
// just check if the customer exists in your Map
0 голосов
/ 28 апреля 2019

использовать customerPosition метод для проверки существует Customer

public void addTransaction(String customerName, Double transactionValue) {
    final int index = customerPosition(customerName);
    if(index == -1) {
        System.out.println("Customer with name " + customerName + " not found");
    } else {
        customers.get(index).addTransaction(transactionValue);
    }
}
0 голосов
/ 28 апреля 2019

Даже если бы вы могли использовать какой-то мутантный вариант indexOf, это не улучшило бы эффективность.Чтобы найти данные в несортированном массиве, вы должны просмотреть массив и найти его.Это присуще выбору массива для хранения данных.

Но почему вы вообще используете customerExists?customerPosition выполняет точно такую ​​же логику.Вам нужно только позвонить один раз. 

   int pos = customerPosition(...);
   if (pos >= 0) 
      customers.get(pos).addTransaction(...);
   else 
      ... does not exist ...
...