Conditional statement:dead code

Question

I'm working on a JavaEE application and I have the following method:

public String alterar_data_ato_med (int cod_ato, GregorianCalendar nova_data) {
    AtoMedico a=em.find(AtoMedico.class,cod_ato);
    Medico m=a.getmedico();
    Utente u=a.getutente();
    GregorianCalendar today=new GregorianCalendar();
    if(a==null){
        return "Ato Médico inexistente!";
    }else{
        if(m.getAgenda_atos().contains(nova_data)||m.getAgenda_consultas().contains(nova_data)){
            return "Médico indisponível";
        }else{
            if(u.getAgenda().contains(nova_data)||nova_data.before(today)){
                return "Data indisponível!";
            }else{
                GregorianCalendar antiga_data=a.getData_ato_med();
                a.setData_ato_med(nova_data);
                m.getAgenda_atos().remove(antiga_data);
                u.getAgenda().remove(antiga_data);
                return "Data do ato médico alterada!";
            }
        }
    }
}

The first if-else statement appears to be 'dead code'. Can please someone help me understant why?

the first one: `if(a==null){return "Ato médico inexistente!";}else{...` — Ghost, Jan 09 '15 at 15:25
also FYI, all the 'else' are useless since you are returning a value in all the if cases — Hacketo, Jan 09 '15 at 15:46

score 11 · Accepted Answer · edited Jan 09 '15 at 15:34

11

a cannot be null at the first if statement. If it were, your code would throw a NullPointerException at the Medico m=a.getmedico() line.

edited Jan 09 '15 at 15:34

Ascalonian

14,409
18
71
103

answered Jan 09 '15 at 15:22

user1717259

2,717
6
30
44

wassgren · Answer 2 · 2015-01-09T15:41:39.603

Well, a can never be null when it reaches the if-statement. Therefore the null check is not needed and the code in the if-statement can never be executed - hence it is dead.

AtoMedico a=em.find(AtoMedico.class,cod_ato); // At this point a may be null...

Medico m=a.getmedico(); // If a is null here, there will be a NullPointerException

// And you will not reach this code down here...

So, what you should do instead is something like:

AtoMedico a=em.find(AtoMedico.class,cod_ato);

if (a == null) {
    return "Ato Médico inexistente!";
}

// Now, it is safe to continue referencing a
Medico m=a.getmedico();
// and so on...

Or, if you use Java 8 you can use the Optional class instead. This example shows how to throw an exception instead of the return.

AtoMedico a = Optional.ofNullable(em.find(AtoMedico.class,cod_ato))
                      .orElseThrow(() -> new IllegalStateException("Ato Médico inexistente!"));
// Now a is safe to use

And, if you don't want that you can use ifPresent. A nice way of handling null!

Optional<AtoMedico> a = Optional.ofNullable(em.find(AtoMedico.class,cod_ato));
a.ifPresent(ato -> {
    // Do your stuff here...
    Medico m=ato.getmedico();
});

For more info on Optional check out this Oracle tutorial.

Conditional statement:dead code

2 Answers2