1

I have to take few actions depending upon the if condition. Say I have an enum "VoucherType"

Now I have a code, that is executed depending upon the condition:-

private boolean verifyGiveAwayAccounting(GiveAwayMoneyVerificationEvent event) {

    if(event.getVoucherType().equals(VoucherType.GIVE_AWAY_MONEY_ON_SIGNUP)){
        someAction();
    }
    return verifySystemAccountTransaction(event);
}

I have to perform someAction() if the event is of type "GIVE_AWAY_MONEY_ON_SIGNUP" . But I do not have to do anything extra is event type is anything other than "GIVE_AWAY_MONEY_ON_SIGNUP". So when I call this code I set the voucherType to "GIVE_AWAY_MONEY_ON_SIGNUP" and someAction() is executed.

But for any other type of events, I get null pointer exceptions in the if condition as I never set the voucher type as I do not want to do anything special. So in order to avoid the nullPointerException I set the Voucher code to something dummy(other voucherType values) which I never use in any conditions. I there a sophisticated way I can eliminate the nullPointerException without initializing the VoucherType in event?

Hovercraft Full Of Eels
  • 283,665
  • 25
  • 256
  • 373
pune06
  • 123
  • 2
  • 10
  • 1
    if(enum && enum.Football) ? – Jordan Plamondon May 10 '16 at 14:18
  • This will not work. First part of the condition will still throw nullPointerException as it will evaluate to null. – pune06 May 10 '16 at 14:24
  • Why so many down votes. I am new to the community. Please help my mentioning the comments, the reason of downvotes. – pune06 May 10 '16 at 14:28
  • 1
    Sorry but where exactly do you get NPE? And what is `enum` here? If it is some class like `Sports.Football` then it can't throw NPE. Please clarify your code example (also `Class` is not keyword to declare class, don't introduce additional problems in your examples). – Pshemo May 10 '16 at 14:28
  • 2
    Not your down-voter, but you're posting kind-of sort-of Java code (`if (enum.Football)`?? makes no sense. `Class`??) which can confuse. You have a real and serious question, and so please strive to post real and serious code. Yes you can and probably should create a simplified code example, but still it should be real Java code that makes sense. – Hovercraft Full Of Eels May 10 '16 at 14:30
  • 2
    Thanks for your suggestions pshemo and @Hovercraft Full Of Eels - I have changed my question to have real code and be clearer. Hopefully it helps. – pune06 May 10 '16 at 14:51
  • @pune06: Another Hint: Since your are comparing Enums, you could also use == instead of equals which would eliminate one more potential NPE-Source (see: http://stackoverflow.com/questions/1750435/comparing-java-enum-members-or-equals) – OH GOD SPIDERS May 10 '16 at 16:25

6 Answers6

8

you should always test if your object is not null before making any test on this object's attributes.

if(enum != null && enum.Football) {
  //some action
}
Asma
  • 163
  • 6
  • Thanks @Asma - Yes, point is very well taken. But I am looking for a different answer. Is there any alternate way. But the chances of having it are looking grim :) – pune06 May 10 '16 at 14:30
  • 2
    @pune06 Any chance you can explain why you don't want to use this solution? Also could you explain your reasons in question you asked, so others would have chance to read about them before posting similar solution? – Pshemo May 10 '16 at 14:35
  • if enum is your object (by the way you have to avoid giving your variables names that can be keywords) there is no other way to avoid a NPE than testing it. More details about your code and your problem will be useful to help you. – Asma May 10 '16 at 14:40
  • @Pshemo - I have rephrased my question. Added the code also. Hopefully this will help. – pune06 May 10 '16 at 14:54
  • @pune06 and what about this ? if(event != null && event.getVoucherType()!= null && event.getVoucherType().equals(VoucherType.GIVE_AWAY_MONEY_ON_SIGNUP)){ someAction(); } I think there is no more sophisticated way to avoid a null pointer exception. – Asma May 10 '16 at 14:58
3

If event is never null, in this case perhaps you can invert your test :

private boolean verifyGiveAwayAccounting(GiveAwayMoneyVerificationEvent event) {
    if(VoucherType.GIVE_AWAY_MONEY_ON_SIGNUP.equals(event.getVoucherType())){
            someAction();
    }
    return verifySystemAccountTransaction(event);
}

otherwise you should test if event is not null before :

private boolean verifyGiveAwayAccounting(GiveAwayMoneyVerificationEvent event) {
    if(event != null && VoucherType.GIVE_AWAY_MONEY_ON_SIGNUP.equals(event.getVoucherType())){
            someAction();
    }
    return verifySystemAccountTransaction(event);
}
Dams
  • 997
  • 12
  • 28
  • 1
    Because we are actually comparing enums, we could just compare references with == and not use equals at all. That would eliminate further possibilities of NPEs. – OH GOD SPIDERS May 10 '16 at 16:24
  • @911DidBush of course, this is a possibility, but some people prefer the equals method for different reasons. The original code uses the equals method and I kept it. – Dams May 10 '16 at 19:19
1

As defined by WikiBooks:

A NullPointerException is thrown when an application attempts to use an object reference, having the null value. These include: Calling an instance method on the object referred by a null reference.

If you do not instantiate the enum value, it will have a value of null. Thus, the program is attempting to reference an object that contains null, which throws a NullPointerException.

Therefore, no, there is no way to avoid your NullPointerException. You need to instantiate variables before attempting to reference them.

serebit
  • 400
  • 2
  • 13
1

I would check if the enum is not null before attempting to check the value of enum.Football.

    void method(){
        if(enum!=null && enum.Football){
            SomeAction();
        }
    }
Vijay
  • 542
  • 4
  • 15
1

Apart from the already mentioned answers of checking for null, another possibility would be to actually create an extra enum value that represents null (a "dummy value") and use that as the default value:

public enum VoucherType {
    UNDEFINED, 
    GIVE_AWAY_MONEY_ON_SIGNUP,
    //....
    ;
}

Define "UNDEFINED" as default value:

public class Event {
    private VoucherType voucherType = VoucherType.UNDEFINED;

    public Event() {
    }

    public VoucherType getVoucherType() {
        return this.voucherType;
    }

    public void setVoucherType(VoucherType voucherType) {
        if(voucherType==null) {
            throw new IllegalArgumentException(); // make sure that voucher type cannot be set to null
        }
        this.voucherType=voucherType;
    }
}

That way events will never have null as voucherType and instead the enum value UNDEFINED.

Warning: A lot of people would prefer recieving a NullPointerException instead of the above solution to immediatly get feedback when they forget to set the voucherType. With the above solution making the error of forgetting to set the voucherType and not realizing it (Because code throws no error) is much easier.

Also it might force you to check if the voucherType is still UNDEFINED for some operations where the requirement is that it is set to a meaningfull value.

I would actually rather check for null myself, but since you said you want other solutions I thought I post this anyway.

OH GOD SPIDERS
  • 3,091
  • 2
  • 13
  • 16
  • 1
    Default value is an interesting proposal, but in my opinion `throw new IllegalArgumentException();` is more appropriate in the setter – Dams May 10 '16 at 19:28
  • Thanks @911DidBush for your effort- Yes, Currently I am using a DummyValue in my enum, which I don't like :) – pune06 May 11 '16 at 08:56
0

Simply invert the operands of the equals:

if(VoucherType.GIVE_AWAY_MONEY_ON_SIGNUP.equals(event.getVoucherType()))

It will never give you NullPointerException for a null value return of your getVoucherType() method. Of course you must assure that event Object is never null.

Or, as suggested in one of the comments use the == operator, it's possible for enum:

if(VoucherType.GIVE_AWAY_MONEY_ON_SIGNUP == event.getVoucherType())
aschmid
  • 53
  • 6