3

In a loop where thousands of items are being converted from String to int, should one check if a string is empty before calling Integer.parseInt or should directly rely on the NumberFormatException to move to the next?

ragnacode
  • 332
  • 3
  • 13
  • 5
    If you know your strings are going to be *either* empty or valid, then check if it's empty. If your string might be invalid in other ways, you might have to catch `NumberFormatException` anyway. – khelwood Jun 06 '18 at 12:54
  • 1
    It depends on the ratio of empty vs non-empty strings, but I think it would have to be terribly high for the test to make your code more efficient than with the exception handling alone. – Aaron Jun 06 '18 at 12:56
  • 1
    İt is too redundant. Catch is More efficent. – anilkay Jun 06 '18 at 12:56
  • If your string can be null, empty or have letters, it's better to catch NumberFormatException. – Diego Victor de Jesus Jun 06 '18 at 12:59
  • You should do what is clearest. IMHO Exceptions should be reserved for exceptional conditions, if it's a common case, check for it. – Peter Lawrey Jun 06 '18 at 13:09
  • The input will have some empty strings for sure, their exact proportion or distribution is unknown, it could vary wildly. – ragnacode Jun 06 '18 at 13:13

3 Answers3

4

If empty strings are exceptions (i.e. it should not happen) in your data then it is accepted practice to not check them and just let the exception system handle it.

If empty strings are possible (even if rare) and would mean something (e.g. "" -> "0") then you should check.

The bottom line is you should not use exceptions to control program flow.

OldCurmudgeon
  • 64,482
  • 16
  • 119
  • 213
  • 2
    "The bottom line is you should not use exceptions to control program flow." Ehh, I tend to think of `parseInt` et al as kind of a special case. There's no function you can call first (e.g. `willParse`) so in order to avoid throwing the exception, you have to implement quite a few checks yourself - the logic isn't quite as simple as one might think. I tend to think it's easier just to catch the exception. [This question](https://stackoverflow.com/questions/1486077/good-way-to-encapsulate-integer-parseint) has some interesting discussion about it. – Michael Jun 06 '18 at 13:11
  • @Michael - `Integer.parseInt` has been a problem for a long time. It has been discussed in many places. I was hoping only to focus on empty strings here as the OP stated. – OldCurmudgeon Jun 11 '18 at 11:49
1

Well, it all depends:

  1. if each string in the source collection is expected to be a valid integer when parsed then I'd say don't check if it's not empty as it will probably end up hiding a bug.
  2. if you expect some strings to be empty and others to be valid integers then yes you'll need to check if it's not empty before parsing to an integer otherwise you'll get an exception.
  3. if the source collection could contain both valid integers as well as strings that cannot be parsed into an integer, then you'll need to perform some validation prior to parsing the strings.
Ousmane D.
  • 54,915
  • 8
  • 91
  • 126
1

No.

You have to catch the NumberFormatException anyway, so adding an additional check just adds more code a reader has to wade through for no functional benefit.

(I'm assuming from the question you do want to check if it's invalid in any case, and not just check if its empty. If you just want to check whether it's empty and not whether it's generally invalid, then obviously just use isEmpty() and don't catch anything at all!)

Yes, exceptions should not normally be used for control flow - but catching a NumberFormatException to check if a String is a valid int or not is a reasonably well understood exception to this rule.

Michael Berry
  • 70,193
  • 21
  • 157
  • 216
  • It really depends on the usecase/requirement (PS. I am not the one who downvoted). – Jaroslaw Pawlak Jun 06 '18 at 13:04
  • @JaroslawPawlak I see no use case where you'd need an additional check to see if it's empty, if you have to catch the exception anyway (perhpas bar a few odd performance cases, in which case the answer is "benchmark and see".) – Michael Berry Jun 06 '18 at 13:07
  • 1
    Agreed. Jon Skeet himself [doesn't seem to have a problem with it](https://stackoverflow.com/a/1486082/1898563). It's an unfortunately designed API, really (I wish there were a sibling method to verify the string were valid) but it's perfectly idiomatic Java. – Michael Jun 06 '18 at 13:22
  • 1
    @Michael Agreed, in an ideal world there'd be an `isParseable()` method on `Integer` or similar that we'd use instead - but there's not, so this is the only reasonable way. – Michael Berry Jun 06 '18 at 13:24