Scope fail
-
Hey guys, i'm a software developer from Romania, and I've been a lurker here for quite a while. Currently, i'm working on an Android project, and, well, this morning I've caused a wtf fail. Thankfully, I realized it immediately (i.e. after the first NullPointerException) and quickly fixed it. However, I could not resist posting this here, as it's a beginner's mistake, and I shouldn't have done it in the first place (shame on me). Variable and method names changed, and the original class was much, much larger
public class CategoryProducts extends ListActivity {
private RangeSeekBar seekBar;/\*Dummy initializers that receive their normal values in the constructor\*/ private int minPrice = 0; private int maxPrice = 100000; private void initFilterView() { if(seekBar == null) { RangeSeekBar seekBar = new RangeSeekBar(minPrice, maxPrice); seekBar.setOnRangeSeekBarChangeListener(new PriceLimitListener()); } ((LinearLayout) filterView.findViewById(R.id.priceSeekBar)).addView(seekBar); }}
The explanation (if you haven't spot it already) is that seekBar gets declared as local in the "if" of the initFilterView method, and visible only in that scope. The local "seekBar" and the global "seekBar" are in fact 2 different vars, and nobody guarantees that the global "seekBar" will not be null (which happens in this case). The correct code was
...(same as above)
if(seekBar == null) {
seekBar = new RangeSeekBar(minPrice, maxPrice);
seekBar.setOnRangeSeekBarChangeListener(new PriceLimitListener());
}
...(same as above) -
Hey guys, i'm a software developer from Romania, and I've been a lurker here for quite a while. Currently, i'm working on an Android project, and, well, this morning I've caused a wtf fail. Thankfully, I realized it immediately (i.e. after the first NullPointerException) and quickly fixed it. However, I could not resist posting this here, as it's a beginner's mistake, and I shouldn't have done it in the first place (shame on me). Variable and method names changed, and the original class was much, much larger
public class CategoryProducts extends ListActivity {
private RangeSeekBar seekBar;/\*Dummy initializers that receive their normal values in the constructor\*/ private int minPrice = 0; private int maxPrice = 100000; private void initFilterView() { if(seekBar == null) { RangeSeekBar seekBar = new RangeSeekBar(minPrice, maxPrice); seekBar.setOnRangeSeekBarChangeListener(new PriceLimitListener()); } ((LinearLayout) filterView.findViewById(R.id.priceSeekBar)).addView(seekBar); }}
The explanation (if you haven't spot it already) is that seekBar gets declared as local in the "if" of the initFilterView method, and visible only in that scope. The local "seekBar" and the global "seekBar" are in fact 2 different vars, and nobody guarantees that the global "seekBar" will not be null (which happens in this case). The correct code was
...(same as above)
if(seekBar == null) {
seekBar = new RangeSeekBar(minPrice, maxPrice);
seekBar.setOnRangeSeekBarChangeListener(new PriceLimitListener());
}
...(same as above)My vote of 5, to accept your mistake. Newton's unspecified law: No code in world is bug free!
-
Hey guys, i'm a software developer from Romania, and I've been a lurker here for quite a while. Currently, i'm working on an Android project, and, well, this morning I've caused a wtf fail. Thankfully, I realized it immediately (i.e. after the first NullPointerException) and quickly fixed it. However, I could not resist posting this here, as it's a beginner's mistake, and I shouldn't have done it in the first place (shame on me). Variable and method names changed, and the original class was much, much larger
public class CategoryProducts extends ListActivity {
private RangeSeekBar seekBar;/\*Dummy initializers that receive their normal values in the constructor\*/ private int minPrice = 0; private int maxPrice = 100000; private void initFilterView() { if(seekBar == null) { RangeSeekBar seekBar = new RangeSeekBar(minPrice, maxPrice); seekBar.setOnRangeSeekBarChangeListener(new PriceLimitListener()); } ((LinearLayout) filterView.findViewById(R.id.priceSeekBar)).addView(seekBar); }}
The explanation (if you haven't spot it already) is that seekBar gets declared as local in the "if" of the initFilterView method, and visible only in that scope. The local "seekBar" and the global "seekBar" are in fact 2 different vars, and nobody guarantees that the global "seekBar" will not be null (which happens in this case). The correct code was
...(same as above)
if(seekBar == null) {
seekBar = new RangeSeekBar(minPrice, maxPrice);
seekBar.setOnRangeSeekBarChangeListener(new PriceLimitListener());
}
...(same as above)Interesting! How did that compile without an error: "A local variable named 'seekBar' cannot be declared in this scope because it would give a different meaning to 'seekBar ', which is already used in a 'parent or current' scope to denote something else" Because it won't for me!
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
-
Interesting! How did that compile without an error: "A local variable named 'seekBar' cannot be declared in this scope because it would give a different meaning to 'seekBar ', which is already used in a 'parent or current' scope to denote something else" Because it won't for me!
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
EDIT: I believe you've tried this in VS, I dunno if Eclipse has that sort of stuff :laugh: I can also post pics ;P
-
EDIT: I believe you've tried this in VS, I dunno if Eclipse has that sort of stuff :laugh: I can also post pics ;P
Now I am starting to believe that world is moving to .net, :thumbsup: God save JAVA :laugh:
-
Now I am starting to believe that world is moving to .net, :thumbsup: God save JAVA :laugh:
Yeah, although Java was the first language I've learned and I do still love it with all my heart, after I worked with .NET I've started to understand why people are moving to it. I think VS2010 Pro is a beauty of an IDE. I didn't get 2011 yet, but from what I've read around here, people aren't really satisfied with it :laugh:
-
Interesting! How did that compile without an error: "A local variable named 'seekBar' cannot be declared in this scope because it would give a different meaning to 'seekBar ', which is already used in a 'parent or current' scope to denote something else" Because it won't for me!
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
-
Most languages other than C# allow this sort of thing. C, C++, and (apparently) Java all do. I'm not sure what the legitimate case for this sort of thing is, though. It seems like a potential trap to me, and little more.
Agreed - variable masking is a bad idea, it leads to some very hard to spot bugs.
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
