2009-06-12 26 views
15

Estoy tratando de averiguar la mejor manera de buscar un cliente en un ArrayList por su número de Id. El código a continuación no está funcionando; el compilador me dice que me falta una declaración return.búsqueda en java ArrayList

Customer findCustomerByid(int id){ 
    boolean exist=false; 

    if(this.customers.isEmpty()) { 
     return null; 
    } 

    for(int i=0;i<this.customers.size();i++) { 
     if(this.customers.get(i).getId() == id) { 
      exist=true; 
      break; 
     } 

     if(exist) { 
      return this.customers.get(id); 
     } else { 
      return this.customers.get(id); 
     } 
    } 

} 

//the customer class is something like that 
public class Customer { 
    //attributes 
    int id; 
    int tel; 
    String fname; 
    String lname; 
    String resgistrationDate; 
} 
+12

Una de las principales Buenas Prácticas de Java es SIEMPRE usar llaves incluso si el bloque tiene solo una oración para evitar problemas como el suyo –

Respuesta

16

El compilador se queja porque actualmente tiene el bloque 'if (exist)' dentro de su bucle for. Necesita estar fuera de eso.

for(int i=0;i<this.customers.size();i++){ 
     if(this.customers.get(i).getId() == id){ 
      exist=true; 
      break; 
     } 
} 

if(exist) { 
    return this.customers.get(id); 
} else { 
    return this.customers.get(id); 
} 

Dicho esto, hay mejores formas de realizar esta búsqueda. Personalmente, si estuviera usando una ArrayList, mi solución se parecería a la que Jon Skeet publicó.

+0

Sí, lo reformaté y ahora es más obvio –

+0

Sospecho que el código aún está roto (nota el argumento para los clientes. obtener al final). ¡El hecho de que también tenga un "si/no" donde ambas ramas tienen el mismo código también es muy sospechoso! –

+0

Estoy de acuerdo, Jon. Además, ciertamente hay mejores formas de realizar la búsqueda (como han indicado otros muchos carteles). –

10
Customer findCustomerByid(int id){ 
    for (int i=0; i<this.customers.size(); i++) { 
     Customer customer = this.customers.get(i); 
     if (customer.getId() == id){ 
      return customer; 
     } 
    } 
    return null; // no Customer found with this ID; maybe throw an exception 
} 
+0

Mudanza si la declaración no es suficiente para arreglar el método. Su solución es preferida. +1 – waxwing

+0

Estoy muy de acuerdo. +1 –

2

que se está perdiendo la instrucción de retorno, porque si el tamaño de su lista es 0, el bucle no se ejecutará, por lo tanto, si el nunca va a funcionar, y por lo tanto nunca volverá.

Mueva la instrucción if fuera del ciclo.

48

Otros han señalado el error en su código actual, pero me gustaría dar dos pasos más. En primer lugar, asumiendo que usted está utilizando Java 1.5+, se puede lograr una mayor legibilidad mediante el bucle for mejorado :

Customer findCustomerByid(int id){  
    for (Customer customer : customers) { 
     if (customer.getId() == id) { 
      return customer; 
     } 
    } 
    return null; 
} 

Esto también ha eliminado el micro-optimización de volver null antes de bucle - Dudo que usted' Obtendré algún beneficio de eso, y es más código. Del mismo modo, eliminé la bandera exists: volver tan pronto como sepa la respuesta simplifica el código.

Tenga en cuenta que en su código original I piensa que tenía un error. Después de haber encontrado que el cliente en el índice i tenía la ID correcta, luego devolvió el cliente al índice id - Dudo que esto sea realmente lo que usted pretendía.

En segundo lugar, si vas a hacer muchas búsquedas por ID, ¿has considerado poner a tus clientes en un Map<Integer, Customer>?

16

personalmente rara vez escribir bucles mí ahora cuando puedo salir con la suya ... Yo uso las Jakarta Commons libs:

Customer findCustomerByid(final int id){ 
    return (Customer) CollectionUtils.find(customers, new Predicate() { 
     public boolean evaluate(Object arg0) { 
      return ((Customer) arg0).getId()==id; 
     } 
    }); 
} 

Yay! ¡Salvé una línea!

+1

Has guardado una línea por cada vez que escribes una función find() :-) ¡Eso no es para ser estornudado! –

+0

Puedes "guardar" una línea de código cada vez que escribes eso, pero te apuesto a que si observas cómo funciona esa función de búsqueda, verás que está usando un ciclo for. Entonces uno debe preguntarse: ¿realmente ha "guardado" alguna línea o agregado más líneas al código subyacente? ;) – Spider

+2

@Spider La ejecución en tiempo de ejecución es solo una forma de medir el código ... hay otros beneficios de tener código menos repetido. –

2

Incluso si ese tema es bastante antiguo, me gustaría agregar algo. Si sobrescribe equals para usted clases, por lo que compara su getId, puede utilizar:

customer = new Customer(id); 
customers.get(customers.indexOf(customer)); 

Por supuesto, usted tendría que comprobar si existe una IndexOutOfBounds -Exception, que oculd traducirse en un puntero nulo o una personalizada CustomerNotFoundException.

0

Hice algo parecido a eso, el compilador está viendo que su declaración de devolución está en una instrucción If(). Si desea resolver este error, simplemente cree una nueva variable local llamada customerId antes de la instrucción If, luego asigne un valor dentro de la instrucción if. Después de la instrucción if, llame a su declaración de devolución y devuelva cstomerId. De esta manera: customerId

Customer findCustomerByid(int id){ 
boolean exist=false; 

if(this.customers.isEmpty()) { 
    return null; 
} 

for(int i=0;i<this.customers.size();i++) { 
    if(this.customers.get(i).getId() == id) { 
     exist=true; 
     break; 
    } 

    int customerId; 

    if(exist) { 
     customerId = this.customers.get(id); 
    } else { 
     customerId = this.customers.get(id); 
    } 
} 

retorno;

}

1

En Java 8:

Customer findCustomerByid(int id) { 
    return this.customers.stream() 
     .filter(customer -> customer.getId().equals(id)) 
     .findFirst().get(); 
} 

También podría ser mejor cambiar el tipo de retorno a Optional<Customer>.