Misplaced warning.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Misplaced warning.

Owen Thomas-2
Hello.

In the following code:

        SynapseOwnerParticipant p=null;
        try{
            p=(SynapseOwnerParticipant)sender;
        }catch(ClassCastException ex){/*Do nothing.*/}
        if(p==this.getPrincipledQuale()){
            p.reactToEngagementMessage(subscription,claimant,citation,sequence);
        }

The IDE shows a warning about the possibility that I could be dereferencing a null pointer on the line where the call to reactToEngagementMessage is assigned to the variable.

Clearly to me, the code shows that the variable p will not be null, so I can't see a reason why this warning would appear. I can add p!=null&& to the condition and the warning goes away. The getPrincipledQuale method is final, and it returns the value of a final field which is set in a constructor from a parameter. If the parameter is set to null, an exception will be thrown from the constructor. Hence, in this instance, the extra expression is redundant lint that makes my code look ugly.

Generally I like the warning; it has appeared in other instances where I found its advice was timely. I don't want to turn off the warning, and I don't really want to add a SuppressWarnings annotation above the method's declaration because it looks even uglier.

Any suggestions?
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Misplaced warning.

Sean Carrick-4

Owen,

The compiler sees that you assigned `null` to the variable `SynapseOwnerParticipant` prior to the `try` block. Therefore, your variable `p` IS `null`. If the program were to blow up in your assignment within the `try` block, `p` WOULD be `null` in the `catch` block. This is a valid warning.

To stop this compile-time error from occuring, you should do the following:

SynapseOwnerParticipant p = new SynapseOwnerParticipant();

By doing this, even though you will not be using the variable `p` in this form, you will not have the compile-time warning about dereferencing a null pointer. Whenever possible, you should avoid assigning `null` to a variable for this purpose. After all, the warnings are attempting to lead you toward better coding practices.

Best of luck!

Sean Carrick

VP Software Development, Integrity Solutions


On 04/18/2017 06:37 AM, Owen Thomas wrote:
Hello.

In the following code:

        SynapseOwnerParticipant p=null;
        try{
            p=(SynapseOwnerParticipant)sender;
        }catch(ClassCastException ex){/*Do nothing.*/}
        if(p==this.getPrincipledQuale()){
            p.reactToEngagementMessage(subscription,claimant,citation,sequence);
        }

The IDE shows a warning about the possibility that I could be dereferencing a null pointer on the line where the call to reactToEngagementMessage is assigned to the variable.

Clearly to me, the code shows that the variable p will not be null, so I can't see a reason why this warning would appear. I can add p!=null&& to the condition and the warning goes away. The getPrincipledQuale method is final, and it returns the value of a final field which is set in a constructor from a parameter. If the parameter is set to null, an exception will be thrown from the constructor. Hence, in this instance, the extra expression is redundant lint that makes my code look ugly.

Generally I like the warning; it has appeared in other instances where I found its advice was timely. I don't want to turn off the warning, and I don't really want to add a SuppressWarnings annotation above the method's declaration because it looks even uglier.

Any suggestions?

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Misplaced warning.

Owen Thomas-2
Thanks for your input Sean.

I think the IDE warning is good to know generally, and although your solution may work, it would mean that I would have to code a no-argument constructor "plug" for this type. Even if I didn't hitherto have a need for a no-argument constructor (I don't in this circumstance so your solution would work in theory), I fear that appropriating this pattern in all instances where my code does something similar would bloat my code and that it would impose a run-time cost.

Sincerely thank you for the suggestion - it didn't occur to me as a consideration before you suggested it, and the input of others always helps me to be aware of alternatives.


On 21 April 2017 at 00:02, Sean Carrick <[hidden email]> wrote:

Owen,

The compiler sees that you assigned `null` to the variable `SynapseOwnerParticipant` prior to the `try` block. Therefore, your variable `p` IS `null`. If the program were to blow up in your assignment within the `try` block, `p` WOULD be `null` in the `catch` block. This is a valid warning.

To stop this compile-time error from occuring, you should do the following:

SynapseOwnerParticipant p = new SynapseOwnerParticipant();

By doing this, even though you will not be using the variable `p` in this form, you will not have the compile-time warning about dereferencing a null pointer. Whenever possible, you should avoid assigning `null` to a variable for this purpose. After all, the warnings are attempting to lead you toward better coding practices.

Best of luck!

Sean Carrick

VP Software Development, Integrity Solutions


On 04/18/2017 06:37 AM, Owen Thomas wrote:
Hello.

In the following code:

        SynapseOwnerParticipant p=null;
        try{
            p=(SynapseOwnerParticipant)sender;
        }catch(ClassCastException ex){/*Do nothing.*/}
        if(p==this.getPrincipledQuale()){
            p.reactToEngagementMessage(subscription,claimant,citation,sequence);
        }

The IDE shows a warning about the possibility that I could be dereferencing a null pointer on the line where the call to reactToEngagementMessage is assigned to the variable.

Clearly to me, the code shows that the variable p will not be null, so I can't see a reason why this warning would appear. I can add p!=null&& to the condition and the warning goes away. The getPrincipledQuale method is final, and it returns the value of a final field which is set in a constructor from a parameter. If the parameter is set to null, an exception will be thrown from the constructor. Hence, in this instance, the extra expression is redundant lint that makes my code look ugly.

Generally I like the warning; it has appeared in other instances where I found its advice was timely. I don't want to turn off the warning, and I don't really want to add a SuppressWarnings annotation above the method's declaration because it looks even uglier.

Any suggestions?


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Misplaced warning.

Sean Carrick-4

Owen,

This was no problem at all.

Another alternative that came to me after I sent the last message would be for you to do the following:

SynapseOwnerParticipant p;

try{
         p=(SynapseOwnerParticipant) sender;

}catch(ClassCastException ex){/*Do nothing.*/}
         if(p==this.getPrincipledQuale(
)){
                p.reactToEngagementMessage(
subscription,claimant,citation,sequence);
        }
}

In other words, just don't initialize the variable when you create it. However, since you are accessing that variable in the `catch` block, it may still kick out the "dereferencing null variable" warning. Just another thought on that.

As for "bloat" and "run-time cost", these are really non-issues. Nothing says that your default, no parameter constructor would have to do anything at all. Therefore, if the constructor is empty, it's just a call, which happens at the speed of electricity, which is virtually unmeasurable (<1ms). Furthermore, one or two lines of code, will not bloat out your program in any perceivable way.

The bloat argument brought to mind a programmer that I had on my team back in the late 90s. He refused to comment his code because he didn't want to cause the executable to be any larger than necessary. That absolutely slayed me, since the compiler drops all commented lines out of the code when it compiles it into executable code, thereby comments do not bloat the final executable file. At least in this situation, bloat was a valid thought, but not with the effect that you feared. I mean, `SynapseOwnerParticipant p = new SynapseOwnerParticipant();` is not really that much more than your original assignment to `null`.

Best of luck!

Sean Carrick
VP Software Development, Integrity Solutions

On 04/20/2017 05:31 PM, Owen Thomas wrote:
Thanks for your input Sean.

I think the IDE warning is good to know generally, and although your solution may work, it would mean that I would have to code a no-argument constructor "plug" for this type. Even if I didn't hitherto have a need for a no-argument constructor (I don't in this circumstance so your solution would work in theory), I fear that appropriating this pattern in all instances where my code does something similar would bloat my code and that it would impose a run-time cost.

Sincerely thank you for the suggestion - it didn't occur to me as a consideration before you suggested it, and the input of others always helps me to be aware of alternatives.


On 21 April 2017 at 00:02, Sean Carrick <[hidden email]> wrote:

Owen,

The compiler sees that you assigned `null` to the variable `SynapseOwnerParticipant` prior to the `try` block. Therefore, your variable `p` IS `null`. If the program were to blow up in your assignment within the `try` block, `p` WOULD be `null` in the `catch` block. This is a valid warning.

To stop this compile-time error from occuring, you should do the following:

SynapseOwnerParticipant p = new SynapseOwnerParticipant();

By doing this, even though you will not be using the variable `p` in this form, you will not have the compile-time warning about dereferencing a null pointer. Whenever possible, you should avoid assigning `null` to a variable for this purpose. After all, the warnings are attempting to lead you toward better coding practices.

Best of luck!

Sean Carrick

VP Software Development, Integrity Solutions


On 04/18/2017 06:37 AM, Owen Thomas wrote:
Hello.

In the following code:

        SynapseOwnerParticipant p=null;
        try{
            p=(SynapseOwnerParticipant)sender;
        }catch(ClassCastException ex){/*Do nothing.*/}
        if(p==this.getPrincipledQuale()){
            p.reactToEngagementMessage(subscription,claimant,citation,sequence);
        }

The IDE shows a warning about the possibility that I could be dereferencing a null pointer on the line where the call to reactToEngagementMessage is assigned to the variable.

Clearly to me, the code shows that the variable p will not be null, so I can't see a reason why this warning would appear. I can add p!=null&& to the condition and the warning goes away. The getPrincipledQuale method is final, and it returns the value of a final field which is set in a constructor from a parameter. If the parameter is set to null, an exception will be thrown from the constructor. Hence, in this instance, the extra expression is redundant lint that makes my code look ugly.

Generally I like the warning; it has appeared in other instances where I found its advice was timely. I don't want to turn off the warning, and I don't really want to add a SuppressWarnings annotation above the method's declaration because it looks even uglier.

Any suggestions?



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Misplaced warning.

Owen Thomas-2
I cannot leave p unassigned because that will cause a compile-time error at the if statement. Even though the compiler might be able to presume that p is null, I think the error is reasonable under the circumstances; I prefer explicit over presumptive assignment and hence an unassigned variable should be flagged by the compiler in this way.

On 21 April 2017 at 09:00, Sean Carrick <[hidden email]> wrote:

Owen,

This was no problem at all.

Another alternative that came to me after I sent the last message would be for you to do the following:

SynapseOwnerParticipant p;

try{
         p=(SynapseOwnerParticipant) sender;

}catch(ClassCastException ex){/*Do nothing.*/}
         if(p==this.getPrincipledQuale(
)){
                p.reactToEngagementMessage(
subscription,claimant,citation,sequence);
        }
}

In other words, just don't initialize the variable when you create it. However, since you are accessing that variable in the `catch` block, it may still kick out the "dereferencing null variable" warning. Just another thought on that.

As for "bloat" and "run-time cost", these are really non-issues. Nothing says that your default, no parameter constructor would have to do anything at all. Therefore, if the constructor is empty, it's just a call, which happens at the speed of electricity, which is virtually unmeasurable (<1ms). Furthermore, one or two lines of code, will not bloat out your program in any perceivable way.

The bloat argument brought to mind a programmer that I had on my team back in the late 90s. He refused to comment his code because he didn't want to cause the executable to be any larger than necessary. That absolutely slayed me, since the compiler drops all commented lines out of the code when it compiles it into executable code, thereby comments do not bloat the final executable file. At least in this situation, bloat was a valid thought, but not with the effect that you feared. I mean, `SynapseOwnerParticipant p = new SynapseOwnerParticipant();` is not really that much more than your original assignment to `null`.

Best of luck!

Sean Carrick
VP Software Development, Integrity Solutions

On 04/20/2017 05:31 PM, Owen Thomas wrote:
Thanks for your input Sean.

I think the IDE warning is good to know generally, and although your solution may work, it would mean that I would have to code a no-argument constructor "plug" for this type. Even if I didn't hitherto have a need for a no-argument constructor (I don't in this circumstance so your solution would work in theory), I fear that appropriating this pattern in all instances where my code does something similar would bloat my code and that it would impose a run-time cost.

Sincerely thank you for the suggestion - it didn't occur to me as a consideration before you suggested it, and the input of others always helps me to be aware of alternatives.


On 21 April 2017 at 00:02, Sean Carrick <[hidden email]> wrote:

Owen,

The compiler sees that you assigned `null` to the variable `SynapseOwnerParticipant` prior to the `try` block. Therefore, your variable `p` IS `null`. If the program were to blow up in your assignment within the `try` block, `p` WOULD be `null` in the `catch` block. This is a valid warning.

To stop this compile-time error from occuring, you should do the following:

SynapseOwnerParticipant p = new SynapseOwnerParticipant();

By doing this, even though you will not be using the variable `p` in this form, you will not have the compile-time warning about dereferencing a null pointer. Whenever possible, you should avoid assigning `null` to a variable for this purpose. After all, the warnings are attempting to lead you toward better coding practices.

Best of luck!

Sean Carrick

VP Software Development, Integrity Solutions


On 04/18/2017 06:37 AM, Owen Thomas wrote:
Hello.

In the following code:

        SynapseOwnerParticipant p=null;
        try{
            p=(SynapseOwnerParticipant)sender;
        }catch(ClassCastException ex){/*Do nothing.*/}
        if(p==this.getPrincipledQuale()){
            p.reactToEngagementMessage(subscription,claimant,citation,sequence);
        }

The IDE shows a warning about the possibility that I could be dereferencing a null pointer on the line where the call to reactToEngagementMessage is assigned to the variable.

Clearly to me, the code shows that the variable p will not be null, so I can't see a reason why this warning would appear. I can add p!=null&& to the condition and the warning goes away. The getPrincipledQuale method is final, and it returns the value of a final field which is set in a constructor from a parameter. If the parameter is set to null, an exception will be thrown from the constructor. Hence, in this instance, the extra expression is redundant lint that makes my code look ugly.

Generally I like the warning; it has appeared in other instances where I found its advice was timely. I don't want to turn off the warning, and I don't really want to add a SuppressWarnings annotation above the method's declaration because it looks even uglier.

Any suggestions?




Loading...