The if command in java is not working within a method

Asked

Viewed 963 times

6

I’m a beginner and I’m not getting my code to work as expected with regard to "if/Else".

I am using this command within a method and doing as follows:

private boolean AutentificaSenha(String s) {
    this.aSenha = s;
    boolean cond = this.senha == this.aSenha;
    if(cond) {
        return true;
    } else {
        return false; 
    }
}

My intention is to create a data authentication system to put into practice what I have learned in some handouts.

This method is one of the 3 methods, after the 3 private methods there is one last public method that calls the other 3.

public boolean Autentifica() {     
    System.out.println("Seu nome: ");
    this.aNome = entrada.nextLine();
    System.out.println("Sua senha: ");
    this.aSenha = entrada.nextLine();   
    System.out.println("Seu ID: ");
    this.aId = entrada.nextInt();

    System.out.println("Aguarde");
    boolean cond =
        this.AutentificaNome(this.aNome) &&
        this.AutentificaSenha(this.senha) &&
        this.AutentificaId(this.id);
    if (cond) {
        System.out.println("Logado com sucesso");
        return true;
    } else{
        System.out.println("Falha ao efetuar loguin");
        return false;
    }
}

My problem is that when I run the code, "login failed" always appears regardless of what I put.

P.S. the private variables: nome, senha and id are already defined.

java if
  • 3

    possible duplicate of How to compare Strings in Java?

  • 4

    Your problem is in this.senha == this.aSenha. This does not do what you expect it to do. See the link question in the comment above.

  • 1

    That’s probably it, but I can’t be sure that’s all it is without looking at the other methods. And Carlos, correcting what Victor pointed out, see that it is possible to simplify the posted method to just one line: return s.equals(this.senha);.

  • Be careful as you are overwriting variables that you should not (i.e. if you fix this problem from == vs. equals, then you will have the opposite problem: will start giving "Successfully logged in" even if the data are wrong). For example, in reading you assign this.aSenha, when calling the authentication you use this.senha (and not what you just read) and within the authentication you do this.aSenha = s (= this.senha). I mean, at the end of the day the test will always come true...

  • 1

    bfavaretto and @mgibsonbr, really on cond, fields are checked incoherently. I wrote an answer detailing everything.

  • 1

    @bfavaretto There really are more problems than just the ==. So I removed my closing vote.

Show 1 more comment

2 answers

8


You didn’t give the methods code AutentificaNome and AutentificaId, but I’ll assume they’re analogous to AutentificaSenha.

So I can reconstruct your entire class like this:

import java.util.Scanner;

public class TestaSenha {

    private Scanner entrada;
    private String senha;
    private String nome;
    private int id;
    private String aSenha;
    private String aNome;
    private int aId;

    public TestaSenha(Scanner entrada, String senha, String nome, int id) {
        this.entrada = entrada;
        this.senha = senha;
        this.nome = nome;
        this.id = id;
    }

    private boolean AutentificaNome(String s) {
        this.aNome = s;
        boolean cond = this.nome == this.aNome;
        if(cond) {
            return true;
        } else {
        return false; }
    }

    private boolean AutentificaSenha(String s) {
        this.aSenha = s;
        boolean cond = this.senha == this.aSenha;
        if(cond) {
            return true;
        } else {
        return false; }
    }

    private boolean AutentificaId(int s) {
        this.aId = s;
        boolean cond = this.id == this.aId;
        if(cond) {
            return true;
        } else {
        return false; }
    }

    public boolean Autentifica() {     
        System.out.println("Seu nome: ");
        this.aNome = entrada.nextLine();
        System.out.println("Sua senha: ");
        this.aSenha = entrada.nextLine();   
        System.out.println("Seu ID: ");
        this.aId = entrada.nextInt();

        System.out.println("Aguarde");
        boolean cond =
        this.AutentificaNome(this.aNome) &&
        this.AutentificaSenha(this.senha) &&
        this.AutentificaId(this.id);
        if (cond) {
            System.out.println("Logado com sucesso");
            return true;
        } else{
            System.out.println("Falha ao efetuar loguin");
            return false;
        }
    }
}

Your most serious mistake is that comparing Strings using the operator == will not do what you want. You should use the method equals(). The reason is that the == compares if two variables point to the same object, which will go wrong when you have two Stringwhich, although having the same content, are two different objects. See more details on this question.

In doing str1.equals(str2), true will be returned if both Stringhave the same content, and false otherwise, unless str1 for null, which will result in a NullPointerException. In this case, we can use the following:

Objects.equals(str1, str2);

And don’t forget to add the import necessary:

import java.util.Objects;

So here’s what your code looks like:

private boolean AutentificaSenha(String s) {
    this.aSenha = s;
    boolean cond = Objects.equals(this.senha, this.aSenha);
    if(cond) {
        return true;
    } else {
    return false; }
}

And the same goes for the AutentificaNome, but not for the AutentificaId (for ints are not objects, so you can compare with the == no problem).

We can improve your code a little bit more, after all if the cond is true, will be returned true, and if cond is false, will be returned false. How about then return the cond directly then?

private boolean AutentificaSenha(String s) {
    this.aSenha = s;
    boolean cond = Objects.equals(this.senha, this.aSenha);
    return cond;
}

We are returning cond, whose value is always the result of Objects.equals(this.senha, this.aSenha). So we can simplify even more by directly returning the result of Objects.equals(this.senha, this.aSenha):

private boolean AutentificaSenha(String s) {
    this.aSenha = s;
    return Objects.equals(this.senha, this.aSenha);
}

And the Java naming rules dictate which method names should start with lower-case letters. So let’s put all four of your methods with names starting with lowercase letters.

We can do something similar at your end of your main method too:

public boolean autentifica() {
    System.out.println("Seu nome: ");
    this.aNome = entrada.nextLine();
    System.out.println("Sua senha: ");
    this.aSenha = entrada.nextLine();   
    System.out.println("Seu ID: ");
    this.aId = entrada.nextInt();

    System.out.println("Aguarde");
    boolean cond =
            this.autentificaNome(this.aNome) &&
            this.autentificaSenha(this.senha) &&
            this.autentificaId(this.id);
    if (cond) {
        System.out.println("Logado com sucesso");
    } else {
        System.out.println("Falha ao efetuar login");
    }
    return cond;
}

Since we are looking at your main method, I already took and arranged the spelling of "loguin" for "login" and also gave a better indentation in the definition of the variable cond. In fact, let’s analyze this variable better:

    boolean cond =
            this.autentificaNome(this.aNome) &&
            this.autentificaSenha(this.senha) &&
            this.autentificaId(this.id);

  • You read the variables aNome, aSenha and aId user, but passes to the methods the variables aNome, senha and id. Note the inconsistency between the variables used.

  • In the method autentificaNome, the value of aNome shall be assigned to the aNome (which does nothing) and then compared to nome.

  • In the method autentificaSenha, the value of senha will be assigned to the aSenha, ignoring what the user typed and making the two passwords always equal.

  • With the method autentificaId, happens something similar to autentificaSenha.

Now we come to an important point, the aNome, aSenha and aId are important to be recorded after the login test is performed? Assuming not, then ideally you would eliminate them and use only local variables to the method autentifica(). Also there is no need to change the value of the object fields when you are just testing whether the entered password is correct or not, after all a login attempt, whether successful or not, should not change the login and password or something like that.

So this is your code now:

import java.util.Scanner;
import java.util.Objects;

public class TestaSenha {

    private Scanner entrada;
    private String senha;
    private String nome;
    private int id;

    public TestaSenha(Scanner entrada, String senha, String nome, int id) {
        this.entrada = entrada;
        this.senha = senha;
        this.nome = nome;
        this.id = id;
    }

    private boolean autentificaNome(String s) {
        return Objects.equals(this.nome, s);
    }

    private boolean autentificaSenha(String s) {
        return Objects.equals(this.senha, s);
    }

    private boolean autentificaId(int s) {
        return this.id == s;
    }

    public boolean autentifica() {     
        System.out.println("Seu nome: ");
        String aNome = entrada.nextLine();
        System.out.println("Sua senha: ");
        String aSenha = entrada.nextLine();   
        System.out.println("Seu ID: ");
        int aId = entrada.nextInt();

        System.out.println("Aguarde");
        boolean cond =
                this.autentificaNome(aNome) &&
                this.autentificaSenha(aSenha) &&
                this.autentificaId(aId);
        if (cond) {
            System.out.println("Logado com sucesso");
        } else {
            System.out.println("Falha ao efetuar login");
        }
        return cond;
    }
}

Your class should be functioning properly by now. The only detail is that it is easier to encapsulate the verification of the three conditions together in a method of its own, and so its code stays like this:

import java.util.Scanner;
import java.util.Objects;

public class TestaSenha {

    private Scanner entrada;
    private String senha;
    private String nome;
    private int id;

    public TestaSenha(Scanner entrada, String senha, String nome, int id) {
        this.entrada = entrada;
        this.senha = senha;
        this.nome = nome;
        this.id = id;
    }

    private boolean autentificaNome(String s) {
        return Objects.equals(this.nome, s);
    }

    private boolean autentificaSenha(String s) {
        return Objects.equals(this.senha, s);
    }

    private boolean autentificaId(int s) {
        return this.id == s;
    }

    private boolean autentifica(String n, String s, int i) {
        return this.autentificaNome(n) &&
               this.autentificaSenha(s) &&
               this.autentificaId(i);
    }

    public boolean autentifica() {     
        System.out.println("Seu nome: ");
        String aNome = entrada.nextLine();
        System.out.println("Sua senha: ");
        String aSenha = entrada.nextLine();   
        System.out.println("Seu ID: ");
        int aId = entrada.nextInt();

        System.out.println("Aguarde");
        boolean cond = this.autentifica(aNome, aSenha, aId);
        if (cond) {
            System.out.println("Logado com sucesso");
        } else {
            System.out.println("Falha ao efetuar login");
        }
        return cond;
    }
}
  • 1

    Oh yeah, it’s just that I started reading and you were answering at first like it was an answer.. then I commented before finishing just to go "buying time"

  • 1

    Wow, you taught me a lesson. I was really distracted doing that I ended up confusing the variables I was going to use and I couldn’t keep up with my own line of reasoning. Including the comic of writing "loguin". Anyway you solved my doubt and showed me how it could be easier. Thank you, I’ll pay more attention next time.

4

Simple answer:

Java strings are objects and each has a different reference (memory address), for example this.senha and this.aSenha so testing the refections via operator == always return false. In these cases always use the equals method.

boolean cond = this.senha != null && this.senha.equals(this.aSenha);

I’m guessing you want to make sure some password has been entered.

That’s all !

Browser other questions tagged java if

You are not signed in. Login or sign up in order to post.