True-way solution in Java: parse 2 numbers from 2 strings and then return their sum

Question

Given the code:

public static int sum(String a, String b) /* throws? WHAT? */ {
  int x = Integer.parseInt(a); // throws NumberFormatException
  int y = Integer.parseInt(b); // throws NumberFormatException
  return x + y;
}

Could you tell if it's good Java or not? What I'm talking about is, NumberFormatException is an unchecked exception. You don't have to specify it as part of sum() signature. Moreover, as far as I understand, the idea of unchecked exceptions is just to signal that program's implementation is incorrect, and even more, catching unchecked exceptions is a bad idea, since it's like fixing bad program at runtime.

Would somebody please clarify whether:

1. I should specify NumberFormatException as a part of method's signature.
2. I should define my own checked exception (BadDataException), handle NumberFormatException inside the method and re-throw it as BadDataException.
3. I should define my own checked exception (BadDataException), validate both strings some way like regular expressions and throw my BadDataException if it doesn't match.
4. Your idea?

Update:

Imagine, it's not an open-source framework, that you should use for some reason. You look at method's signature and think - "OK, it never throws". Then, some day, you got an exception. Is it normal?

Update 2:

There are some comments saying my sum(String, String) is a bad design. I do absolutely agree, but for those who believe that original problem would just never appear if we had good design, here's an extra question:

The problem definition is like this: you have a data source where numbers are stored as Strings. This source may be XML file, web page, desktop window with 2 edit boxes, whatever.

Your goal is to implement the logic that takes these 2 Strings, converts them to ints and displays message box saying "the sum is xxx".

No matter what's the approach you use to design/implement this, you'll have these 2 points of inner functionality:

1. A place where you convert String to int
2. A place where you add 2 ints

The primary question of my original post is:

Integer.parseInt() expects correct string to be passed. Whenever you pass a bad string, it means that your program is incorrect (not "your user is an idiot"). You need to implement the piece of code where on one hand you have Integer.parseInt() with MUST semantics and on the other hand you need to be OK with the cases when input is incorrect - SHOULD semantics.

So, briefly: how do I implement SHOULD semantics if I only have MUST libraries.

Answer 1

This is a good question. I wish more people would think about such things.

IMHO, throwing unchecked exceptions is acceptable if you've been passed rubbish parameters.

Generally speaking, you shouldn't throw BadDataException because you shouldn't use Exceptions to control program flow. Exceptions are for the exceptional. Callers to your method can know before they call it if their strings are numbers or not, so passing rubbish in is avoidable and therefore can be considered a programming error, which means it's OK to throw unchecked exceptions.

Regarding declaring throws NumberFormatException - this is not that useful, because few will notice due to NumberFormatException being unchecked. However, IDE's can make use of it and offer to wrap in try/catch correctly. A good option is to use javadoc as well, eg:

/**
 * Adds two string numbers
 * @param a
 * @param b
 * @return
 * @throws NumberFormatException if either of a or b is not an integer
 */
public static int sum(String a, String b) throws NumberFormatException {
    int x = Integer.parseInt(a); 
    int y = Integer.parseInt(b); 
    return x + y;
}

EDITED:
The commenters have made valid points. You need to consider how this will be used and the overall design of your app.

If the method will be used all over the place, and it's important that all callers handle problems, the declare the method as throwing a checked exception (forcing callers to deal with problems), but cluttering the code with try/catch blocks.

If on the other hand we are using this method with data we trust, then declare it as above, because it is not expected to ever explode and you avoid the code clutter of essentially unnecessary try/catch blocks.

Answer 2

In my opinion it would be preferable to handle exception logic as far up as possible. Hence I would prefer the signature

 public static int sum(int a, int b);

With your method signature I would not change anything. Either you are

• Programmatically using incorrect values, where you instead could validate your producer algorithm
• or sending values from e.g., user input, in which case that module should perform the validation

Hence, exception handling in this case becomes a documentation issue.

Answer 3

Number 4. As given, this method should not take strings as parameters it should take integers. In which case (since java wraps instead of overflowing) there's no possibility of an exception.

 x = sum(Integer.parseInt(a), Integer.parseInt(b))

is a lot clearer as to what is meant than x = sum(a, b)

You want the exception to happen as close to the source (input) as possible.

As to options 1-3, you don't define an exception because you expect your callers to assume that otherwise your code can't fail, you define an exception to define what happens under known failure conditions WHICH ARE UNIQUE TO YOUR METHOD. I.e. if you have a method that is a wrapper around another object, and it throws an exception then pass it along. Only if the exception is unique to your method should you throw a custom exception (frex, in your example, if sum was supposed to only return positive results, then checking for that and throwing an exception would be appropriate, if on the other hand java threw an overflow exception instead of wrapping, then you would pass that along, not define it in your signature, rename it, or eat it).

Update in response to update of the question:

So, briefly: how do I implement SHOULD semantics if I only have MUST libraries.

The solution to this is to to wrap the MUST library, and return a SHOULD value. In this case, a function that returns an Integer. Write a function that takes a string and returns an Integer object -- either it works, or it returns null (like guava's Ints.tryParse). Do your validation seperate from your operation, your operation should take ints. Whether your operation gets called with default values when you have invalid input, or you do something else, will depend upon your specs -- most I can say about that, is that it's really unlikely that the place to make that decision is in the operation method.

Answer 4

1. I should specify NumberFormatException as a part of method's signature.

I think so. It's a nice documentation.

2. I should define my own checked exception (BadDataException), handle NumberFormatException inside the method and re-throw it as BadDataException.

Sometimes yes. The checked exceptions are consider to be better in some cases, but working with them is quite a PITA. That's why many frameworks (e.g., Hibernate) use runtime exceptions only.

3. I should define my own checked exception (BadDataException), validate both strings some way like regular expressions and throw my BadDataException if it doesn't match.

Never. More work, less speed (unless you expect throwing the exception to be a rule), and no gain at all.

4. Your idea?

None at all.

Answer 5

Nr 4.

I think I wouldn't change the method at all. I would put a try catch around the calling method or higher in the stack-trace where I'm in a context where I can gracefully recover with business logic from the exception.

I wouldn't certainty do #3 as I deem it overkill.

Answer 6

Assuming that what you are writing is going to be consumed (like as an API) by someone else, then you should go with 1, NumberFormatException is specifically for the purpose of communicating such exceptions and should be used.

Answer 7

1. First you need to ask your self, does the user of my method needs to worry about entering wrong data, or is it expected of him to enter proper data (in this case String). This expectation is also know as design by contract.

2. and 3. Yes you probably should define BadDataException or even better use some of the excising ones like NumberFormatException but rather the leaving the standard message to be show. Catch NumberFormatException in the method and re-throw it with your message, not forgetting to include the original stack trace.

3. It depends on the situation bu I would probably go with re-throwing NumberFormatException with some additional info. And also there must be a javadoc explanation of what are the expected values for String a, String b

Answer 8

Depends a lot on the scenario you are in.

Case 1. Its always you who debug the code and no one else and exception wont cause a bad user experience

Throw the default NumberFormatException

Case2: Code should be extremely maintainable and understandable

Define your own exception and add lot more data for debugging while throwing it.

You dont need regex checks as, its gonna go to exception on bad input anyway.

If it was a production level code, my idea would be to define more than one custom exceptions, like

1. Number format exception
2. Overflow exception
3. Null exception etc...

and deal with all these seperately

Answer 9

1. You may do so, to make it clear that this can happen for incorrect input. It might help someone using your code to remember handling this situation. More specifically, you're making it clear that you don't handle it in the code yourself, or return some specific value instead. Of course, the JavaDoc should make this clear too.
2. Only if you want to force the caller to deal with a checked exception.
3. That seems like overkill. Rely on the parsing to detect bad input.

Overal, a NumberFormaException is unchecked because it is expected that correctly parseable input is provided. Input validation is something you should handle. However, actually parsing the input is the easiest way to do this. You could simply leave your method as it is and warn in the documentation that correct input is expected and anyone calling your function should validate both inputs before using it.

Answer 10

Any exceptional behaviour should be clarified in the documentation. Either it should state that this method returns a special value in case the of failure (like null, by changing the return type to Integer) or case 1 should be used. Having it explicit in the method's signature lets the user ignore it if he ensures correct strings by other means, but it still is obvious that the method doesn't handle this kind of failure by itself.

Answer 11

Answer to your updated question.

Yes it's perfectly normal to get "surprise" exceptions. Think about all the run time errors one got when new to programming.

e.g ArrayIndexOutofBound

Also a common surprise exception from the for each loop.

ConcurrentModificationException or something like that

Answer 12

While I agree with the answer that the runtime exception should be allowed to be percolated, from a design and usability perspective, it would be a good idea to wrap it into a IllegalArgumentException rather than throw it as NumberFormatException. This then makes the contract of your method more clear whereby it declares an illegal argument was passed to it due to which it threw an exception.

Regarding the update to the question "Imagine, it's not an open-source framework, that you should use for some reason. You look at method's signature and think - "OK, it never throws". Then, some day, you got an exception. Is it normal?" the javadoc of your method should always spill out the behavior of your method (pre and post constraints). Think on the lines of say collection interfaces where in if a null is not allowed the javadoc says that a null pointer exception will be thrown although it is never part of the method signature.

Answer 13

As you are talking about good java practice ,in my opinion it is always better

• To handle the unchecked exception then analyze it and through a custom unchecked exception.

• Also while throwing custom unchecked exception you can add the Exception message that your client could understand and also print the stack trace of original exception

• No need to declare custom exception as "throws" as it is unchecked one.

• This way you are not violating the use of what unchecked exceptions are made for, at the same time client of the code would easily understand the reason and solution for the exception .

• Also documenting properly in java-doc is a good practice and helps a lot.

Answer 14

I think it depends on your purpose, but I would document it at a minimum:

/**
 * @return the sum (as an int) of the two strings
 * @throws NumberFormatException if either string can't be converted to an Integer
 */
public static int sum(String a, String b)
  int x = Integer.parseInt(a);
  int y = Integer.parseInt(b);
  return x + y;
}

Or, take a page from the Java source code for the java.lang.Integer class:

public static int parseInt(java.lang.String string) throws java.lang.NumberFormatException;

Answer 15

How about the input validation pattern implemented by Google's 'Guava' library or Apache's 'Validator' library (comparison)?

In my experience, it is considered good practice to validate a function's parameters at the beginning of the function and throw Exceptions where appropriate.

Also, I would consider this question to be largely language independent. The 'good practice' here would apply to all languages that have functions which can take parameters which may or may not be valid.

Answer 16

I think your very first sentence of "Quite a stupid question" is very relevant. Why would you ever write a method with that signature in the first place? Does it even make sense to sum two strings? If the calling method wants to sum two strings, it is the calling method's responsibility to make sure they are valid ints and to convert them before calling the method.

In this example, if the calling method cannot convert the two Strings into an int, it could do several things. It really depends at what layer this summation occurs at. I am assuming the String conversion would be very close to front-end code (if it was done properly), such that case 1. would be the most likely:

1. Set an error message and stop processing or redirect to an error page
2. Return false (ie, it would put the sum into some other object and would not be required to return it)
3. Throw some BadDataException as you are suggesting, but unless the summation of these two numbers is very important, this is overkill, and like mentioned above, this is probably bad design since it implies that the conversion is being done in the wrong place

Answer 17

There are lots of interesting answers to this question. But I still want to add this :

For string parsing, I always prefer to use "regular expressions". The java.util.regex package is there to help us. So I will end up with something like this, that never throws any exception. It's up to me to return a special value if I want to catch some error :

import java.util.regex.Pattern;
import java.util.regex.Matcher;

public static int sum(String a, String b) {
  final String REGEX = "\\d"; // a single digit
  Pattern pattern = Pattern.compile(REGEX);
  Matcher matcher = pattern.matcher(a);
  if (matcher.find()) { x = Integer.matcher.group(); }
  Matcher matcher = pattern.matcher(b);
  if (matcher.find()) { y = Integer.matcher.group(); }
  return x + y;
}

As one can see, the code is just a bit longer, but we can handle what we want (and set default values for x and y, control what happens with else clauses, etc...) We could even write a more general transformation routine, to which we can pass strings, defaut return values, REGEX code to compile, error messages to throw, ...

Hope It was usefull.

Warning : I was not able to test this code, so please excuse eventual syntax problems.

Answer 18

You face this issue because you let user errors propagate too deep into the core of the application and partly also because you abuse Java data types.

You should have a clearer separation between user input validation and business logic, use proper data typing, and this problem will disappear by itself.

The fact is the semantics of Integer.parseInt() are known - it's primary purpose it to parse valid integers. You're missing an explicit user input validation/parsing step.