search

makerbot / ReplicatorG

An open-source gcode interpreter for driving RepRaps, Makerbots, and other similar CNC beasties
http://replicat.org
GNU General Public License v2.0
404 stars 226 forks source link

containsIgnoreCase throws NPE #243

Open larsrc opened 13 years ago

larsrc commented 13 years ago

The implementation of containsIgnoreCase at https://github.com/makerbot/ReplicatorG/blob/master/src/replicatorg/app/ui/MainWindow.java#L1255 throws a NullPointerException:

14/11/2011 19:22:36 [0x0-0x5a05a].org.replicatorg.app[2587] Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException 14/11/2011 19:22:36 [0x0-0x5a05a].org.replicatorg.app[2587] at java.util.regex.Matcher.getTextLength(Matcher.java:1140) 14/11/2011 19:22:36 [0x0-0x5a05a].org.replicatorg.app[2587] at java.util.regex.Matcher.reset(Matcher.java:291) 14/11/2011 19:22:36 [0x0-0x5a05a].org.replicatorg.app[2587] at java.util.regex.Matcher.(Matcher.java:211) 14/11/2011 19:22:36 [0x0-0x5a05a].org.replicatorg.app[2587] at java.util.regex.Pattern.matcher(Pattern.java:888) 14/11/2011 19:22:36 [0x0-0x5a05a].org.replicatorg.app[2587] at replicatorg.app.ui.MainWindow$MachineMenuListener.containsIgnoreCase(MainWindow.java:1256) 14/11/2011 19:22:36 [0x0-0x5a05a].org.replicatorg.app[2587] at replicatorg.app.ui.MainWindow$MachineMenuListener.actionPerformed(MainWindow.java:1276)

This probably happens when there has been a machine with a null name. It gets stored in the prefs, and all attempts at updating it causes a NPE because of the check at line 1277.

I am also curious to hear why you think making a containsIgnoreCase that compiles a regular expression at every call is quicker than the built-in one.

FarMcKon commented 13 years ago

Excellent catch.

I'm not a java programmer by trade, so I'm not familiar with what you are calling 'the built-in one'. A function built into java String? Coming from python/C/C++ land, I'm use to using Regular Expression modules to do that kind of matching. From what you are saying, I guess String has something built-in to do a similar function? When you are use to using a hammer, all problems look like nails, etc.

I'll hit the docs now to see what I can dig up, but if that 'built in one' is something obscure, drop me a note please.

Thanks for pointing out that bug, and sorry if that's prevented you from saving/storing updates. I'll get that fixed and into our repo as soon as I get a chance, and I'll drop you a note when it's fixed.

FarMcKon commented 13 years ago

Hmm. I see a string function for 'compareIgnoreCase', and 'contains', but not containsIgnoreCase function. Based on that (and a quick google search or two) I'm sticking with the 'http://stackoverflow.com/questions/86780/is-the-contains-method-in-java-lang-string-case-sensitive' solution for now. If you can suggest a better solution, I'd be glad to roll it in, or you can just send a pull request and I'll merge in your solution.

larsrc commented 13 years ago

You're right, I was conflating those two methods on String. It's kinda stupid there's not containsIgnoreCase.

Thanks you for the fix, that's a lot easier to read.