Java - a method for not repeat the NIF

Question

I have this method

public static boolean DNIRepetido(String dni1){
        boolean aux=false;

        for (Cliente cliente : cli) {
            if(dni1==cliente.dni){
                aux=true;
            }else{
                aux=false;
                break;
            }
        }
        return aux;
    }

but when I implement it, it doesen't works fine

do{
    System.out.print("DNI: ");
    c.dni=sc.next();
}while((ValidarDNI(c.dni)==false)&&(DNIRepetido(c.dni)==false));

("ValidarDNI" works fine)

NOOOOOOOOOOO.. `==` again :_( – Maroun Jun 12 '13 at 08:34 — Maroun, Jun 12 '13 at 08:34

score 2 · Accepted Answer · answered Jun 12 '13 at 08:33

2

You should compare strings via the equals() method.

if(dni1==cliente.dni){

should be

if(dni1.equals(cliente.dni)){

(you may wish to cater for null references in the above)

The == operator is using reference equality, whereas the equals() method compares the actual objects (i.e. is the array of characters the same - are they the same string).

Note that == will work if you're using interned strings, which directly instantiated strings are.

answered Jun 12 '13 at 08:33

Brian Agnew

268,207
37
334
440

My dream is that a day will pass without any question about `==` vs `equals()`. – Maroun Jun 12 '13 at 08:37
I second that completely – Brian Agnew Jun 12 '13 at 08:38

score 2 · Answer 2 · answered Jun 12 '13 at 08:39

Strings have to be compared for equality (same content) with equals method. It's a frequent mistake.

Also, I would check the DNIRepetido exit condition. I guess you want to exit when you find a repeated DNI. In your case if you find a repeated DNI (and thus set aux = true) the next iteration of the for loop would change that fact:

public static boolean DNIRepetido(String dni1){
        for (Cliente cliente : cli) {
            if(dni1.equals(cliente.dni)){
                return true;
            }
        }
        return false
    }

Ravi K Thapliyal · Answer 3 · 2013-06-12T08:45:35.147

The context isn't clear but

if (dni1==cliente.dni) { // is the wrong way to compare two strings

and you should break on a match; not when it doesn't

if(dni1==cliente.dni){
  aux=true;
  break; // here
} else {
  aux=false;
  // break; // not here
}

Use equals() instead (almost always to compare Objects)

if (dni1 != null && dni1.equals(cliente.getDni())) {
// this compares the actual text content; not the reference

And, I suggest you make your member fields private and use getter methods to access them.

score 0 · Answer 4 · answered Jun 12 '13 at 08:41

Just to add another problem in your code, besides the equals() method for comparing Strings.

When you enter the loop, the first time you'll find a client with a different DNI, you'll quit the loop and won't check for other client DNIs. You should avoid the else part of your code:

public static boolean DNIRepetido(String dni1){
        boolean aux=false;

        for (Cliente cliente : cli) {
            if(dni1.equals(cliente.dni)){
                aux=true;
            }
        }
        return aux;
    }

Java - a method for not repeat the NIF

4 Answers4