41

In a certain try block, I have two String variables which could cause NumberFormatException when I user Integer.parseInt(string1) andInteger.parseInt(string2). The question is, if I catch an exception, how to know which string is the troublemaker? I need to get the troublemaker's variable name.

Here is some example code:

public class test {
    public static void main(String[] args) {
        try {
            String string1 = "fdsa";
            String string2 = "fbbbb";
            Integer.parseInt(string1);
            Integer.parseInt(string2);
        } catch (NumberFormatException e) {
            e.printStackTrace();
        }
        }
    }

And the method e.printStackTrace() doesn't tell me the variable name; it just tells me the content of the troublemaker.

java.lang.NumberFormatException: For input string: "fdsa" at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65) at java.lang.Integer.parseInt(Integer.java:580) at java.lang.Integer.parseInt(Integer.java:615) at test.main(test.java:9) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

Process finished with exit code 0

The reason that I need to know the variable's name is that I need to prompt the user what's going on. For instance, tell the user that string1 is wrong by using

System.out.println(troubleMakerName + "is wrong!")

In my Requirements, the user should input

fd=(fileName,maxLength,minLength)

then I will analyse the input string and create some responses. So I'd like to check whether the maxLength and minLength will throw NumberFormatException. In this case, if minLength has something wrong, then I need to prompt the user that the minLength is wrong.

Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
guo
  • 9,674
  • 9
  • 41
  • 79
  • 4
    1) You can look at the exception to see which value is complained about (e.g. message, line number in stack trace); 2) you can put the `parseInt`s in separate `try`/`catch` blocks. – Andy Turner Jul 28 '16 at 07:47
  • @AndyTurner Let's go with way 1. I don't know how to "look at the exception". – guo Jul 28 '16 at 07:49
  • 4
    `e.printStackTrace()`, or whatever your exception variable is called. But note that this can't tell you the name of the variable programmatically, since variable names don't exist at runtime. – Andy Turner Jul 28 '16 at 07:50
  • The last line in the stack trace gives you the line on which the error occurs. In this case, `AppMain.java:147`. – Polygnome Jul 28 '16 at 07:58
  • 1
    @AndyTurner, 1) small code deletions/insertions in this class will break down the whole algorithm of line finding. 2) it looks like code duplication – Andrew Tobilko Jul 28 '16 at 08:00
  • @Polygnome Kindly look at my update, I don't need to know where is the failure code, I just want to know the name of troublemaker. – guo Jul 28 '16 at 08:01
  • @guo: Thats the same thing. If you look at that line in your source code, you should see the trouble maker immediately. If you don't, then you should refactor your code so that you do. – Polygnome Jul 28 '16 at 08:02
  • @guo: The user should *never* be bothered with *internal* things like names of variables. its meaningless for him, and should be so. Provide a meaningful eror message that can be understood by non-technical people. – Polygnome Jul 28 '16 at 08:04
  • @guo in general you can't know what the "troublemaker variable name" is because 1) (as I said before) variable names don't exist at runtime; 2) the parameter to `Integer.parseInt` doesn't have to be a variable: it could be `Integer.parseInt("frobnitz")`, or `Integer.parseInt(string1 + "frobnitz")`. – Andy Turner Jul 28 '16 at 08:04
  • @AndrewTobilko sure, it duplicates code. But how would you do it otherwise? – Andy Turner Jul 28 '16 at 08:05
  • 2
    Possible duplicate of [What is a debugger and how can it help me diagnose problems?](http://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – Raedwald Jul 28 '16 at 08:39
  • I think @AndyTurner NAILED IT. The idea is that the debugger will help point out the culprit (in this case your variable). However, `In this case, if minLength(VALUE of varible fd) has something wrong, then I need to prompt user the minLength is wrong.` The best way would be to keep it simple KISS and print the variables in the `catch` block as per the answers below. In your scenario (Additional 3), the `fd` variable will only hold the values but if you want to check the `minLength` then it's easier to create a method to handle that. – Arty Jul 28 '16 at 08:55
  • Generally speaking, don't use exceptions for expected program operation. In this case, you can use an if-statement to validate the number before passing it to `parseInt`. Exceptions are expensive and can be misleading (as we see here), simple conditionals are cheap and straightforward. – WannabeCoder Jul 28 '16 at 17:23
  • 1
    @WannabeCoder That is correct if there is an easy way to validate the input. with integers, there isn't, at least not in the standard library. So using parseInt and catching the experssion here is actually the most straight-forward and less error-prone way to solve the problem. There is little reason to include Guava or apache Commons if the *only* thing you need from it is the ability to validate integers. Don't overengineer. – Polygnome Jul 28 '16 at 17:59

9 Answers9

70

You are having an XY-Problem.

You don't want to read the actual variable name. You want to be able to validate input and give reasonable error messages to your user.

String fileName, maxLengthInput, minLengthInput;
int maxLength, minLength;

List<String> errors = new ArrayList<>();

try {
    maxLength = Integer.parseInt(maxlengthInput);
} catch (NumberFormatException nfe) {
    errors.add("Invalid input for maximum length, input is not a number");
}

try {
    minLength = Integer.parseInt(minlengthInput);
} catch (NumberFormatException nfe) {
    errors.add("Invalid input for minimum length, input is not a number");
}

// show all error strings to the user

Not throwing the exceptions directly but collecting them allows you to notify the user about all invalid inputs at once (maybe highlight the related fields with red color) instead of having them fix one input, trying to submit again, and then to see that another input is also wrong.

Instead of Strings you could user your own data struture containing information of the related field etc., but that quickly gets out of scope. The main gist is: use two try-catch blocks, and you are able to differentiate which field is errorneous.

If more inputs are involved, you can refactor this into a loop.

Community
  • 1
  • 1
Polygnome
  • 7,639
  • 2
  • 37
  • 57
  • 9
    imagine you have 10 such variables which need to check out, would you add 10 `try-catch` blocks in that case? it looks like code duplication – Andrew Tobilko Jul 28 '16 at 13:48
  • 1
    For those who use Guava, use Ints#tryParse may save you some exception performance cost. – Jean-François Savard Jul 28 '16 at 14:34
  • 2
    @AndrewTobilko - That's why we also have generic programming. – T.E.D. Jul 28 '16 at 15:01
  • 8
    @AndrewTobilko No. Typically you would get those inputs from the UI. That means you already hade some data structure in place that you can plug this all in to. So I would only loop over the fields I need, apply the neccesary validator (which would be abstracted away) and collect information about all fields that fail validation and show the proper information to the user. But I wouldn't design such an architecture for just two fields and two values. Simplicty is sometimes best. But if you read my answer correctly, i explicitly stated that I would use loops of more fields are involved. – Polygnome Jul 28 '16 at 16:16
  • @Jean-FrançoisSavard The performance you save at that point would easily be outmatched by the hit you take for all the additional classes that have to be loaded. Adding libraries prematurely only bloats the classpath and doesn't add much value. – Polygnome Jul 28 '16 at 16:23
  • 3
    "For those who use Guava" – Jean-François Savard Jul 28 '16 at 16:25
  • 1
    This is screaming for method. – JimmyJames Jul 28 '16 at 21:06
  • "XY-Problem": http://meta.stackexchange.com/questions/66377/what-is-the-xy-problem – Scott Wegner Aug 04 '16 at 16:33
  • @ScottWegner Thanks, I've added the link to the answer. – Polygnome Aug 04 '16 at 17:12
10

Use 2 seperate try,catch blocks to parse two inputs for each variables. Then generate the sanity check message inside each catch block.

        String string1 = "fdsa";
        String string2 = "fbbbb";
        try {
            Integer.parseInt(string1);
        } catch (NumberFormatException e) {
            e.printStackTrace();
            **//Please provide a valid integer for string1**
        }
        try {
            Integer.parseInt(string2 );
        } catch (NumberFormatException e) {
            e.printStackTrace();
           **//Please provide a valid integer for string2** 
        }
Chand Priyankara
  • 6,739
  • 2
  • 40
  • 63
6

I would like to write own parseInt method:

public static int parseInt(String s, 
                           Supplier<? extends RuntimeException> supplier) {
    try {
        return Integer.parseInt(s);
    } catch (NumberFormatException e) {
        throw (RuntimeException)supplier.get().initCause(e);
    }
}

As I know, we can't read a variable name by reflection, so I just pass a String literal instead:

parseInt(string1, () -> new NumberFormatException("string1"));

I will leave the origin answer and provide a version that has discussed in the comments. But for now, I am confused why a Supplier is an overkill.

public static int parseInt(String s, String message) {
    try {
        return Integer.parseInt(s);
    } catch (NumberFormatException e) {
        throw (NumberFormatException)new NumberFormatException(message).initCause(e);
        // throw new IllegalArgumentException(message, e);
    }
}

Its call looks like

parseInt(string1, "string1");
Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
  • Could you tell me what is `() -> new NumberFormatException("string1")`? – guo Jul 28 '16 at 08:25
  • 1
    @guo, are you familiar with java 8? that is a lambda expression that returns `Supplier extends Exception>` on which we can invoke `get()` to get an exception object – Andrew Tobilko Jul 28 '16 at 08:50
  • @guo, of course, you could do it without java 8 like ` int parseInt(String s, E e)` and will throw `e` – Andrew Tobilko Jul 28 '16 at 08:53
  • @guo, the main problem is here - there are no ways to get a variable name in Java, so we have to hardcode a string literal as a message – Andrew Tobilko Jul 28 '16 at 08:57
  • 7
    Java8 supplier is overkill, you can just use `parseInt(String maybeNumber, String variableName)` and then `throw new IllegalArgumentException(variableName + " is not a number!", e);`. This also keeps the stack trace cleaner. – Walter Laan Jul 28 '16 at 10:43
  • @WalterLaan Or better yet; ... String message and throw new IllegalArgumentException(message) – Taemyr Jul 28 '16 at 10:57
  • @WalterLaan, I provide to a user more flexible way than you showed, he will be able to throw an exception of any type with any message inside – Andrew Tobilko Jul 28 '16 at 11:07
  • There is no point in throwing multiple exception type for a same unexpected behavior (wrong number format). I downvoted because I also think Supplier is overkill here. – Jean-François Savard Jul 28 '16 at 14:38
  • Part of the point Walter was making is that your example discards the `NumberFormatException` so that you lose the stack trace and message. Any time you rethrow an exception, you should always pass the old exception as the cause to preserve it. Your example also forces the code which calls the method to `catch(Exception e)` which is pretty bad. – Radiodef Jul 28 '16 at 15:27
  • @Jean-FrançoisSavard, I can't agree with you. the fact that user made a mistake is more important for him than the fact that the string cannot be converted to an int. So, there may be any type of custom exceptions (like IncorrectUserInputException) that extend Exception. Or should I be limited only unchecked exceptions to prevent handling Exception every time when I use the method? If yes, I agree and will change. – Andrew Tobilko Jul 28 '16 at 15:30
  • @Radiodef, I am thinking about it too. Will RuntimeException be more convenient? To prevent constant handling. – Andrew Tobilko Jul 28 '16 at 15:33
  • *"the fact that user made a mistake is more important for him than the fact that the string cannot be converted to an int. So, there may be any type of custom exceptions..."* - But in this case, the mistake is always that the string cannot be converted to an int since you catch a NumberFormatException. – Jean-François Savard Jul 28 '16 at 15:41
  • @Jean-FrançoisSavard, yes, we know why the error occurs, but not we have to decide which exception should be thrown, I leave it for API user. For example, look at [`Optional#orElseThrow`](https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html), it would just throw NPE, but it doesn't do that, why? – Andrew Tobilko Jul 28 '16 at 16:01
  • I really don't see how a method named parseInt should throw an exception other than NumberFormatException. Specially that all it does is trying to call Integer#parseInt which throws NumberFormatException. Only point of wrapping the call would be to change the error message with the variable name as needed, but no need of supplier for that. – Jean-François Savard Jul 28 '16 at 17:29
  • @AndrewTobilko "Will RuntimeException be more convenient? To prevent constant handling." it would be the OPPOSITE of convenient. Any validation exception should be trivial to handle. As such, they should also always be handled. Suggesting the application has to scream and throw a fit if input is slightly wrong is just ludicrous. If you have a variety of reasons for validation to fail, then create your own ValidationException and throw that in the code that does the validation itself. Then catch it and do something appropriate. – VLAZ Jul 28 '16 at 20:13
  • 1
    This is definitely the right answer except that the Supplier is almost always going to be overkill. I would have either the variable name or the error message I would present on an error. Other than that, I can't believe this doesn't beat the copy-paste answer. – JimmyJames Jul 28 '16 at 21:04
4

Before you perform the operation you can write a simple isInteger() function that can return you a boolean. A good implementation can be found on this thread. This uses the radix of the value and iterates if it is an int and is quite handy. Determine if a String is an Integer in Java A simple if conditional then can find which value is rogue in the argument

Ramachandran.A.G
  • 4,788
  • 1
  • 12
  • 24
3

Just use another try catch for the other statement

public class test 
{
    public static void main(String[] args) 
    {
        String string1 = "fdsa";
        String string2 = "fbbbb";

        try 
        {
            Integer.parseInt(string1);
        } 
        catch (NumberFormatException e) 
        {
            System.out.println("string1 is the culprit");
            e.printStackTrace();
        }

        try 
        {
            Integer.parseInt(string2);
        } 
        catch (NumberFormatException e) 
        {
            System.out.println("string2 is the culprit");
            e.printStackTrace();
        }
    }
}
Black
  • 18,150
  • 39
  • 158
  • 271
2

You could create a method with a try/catch that returns the value you assign or prints to console that there was a problem, this way you could accommodate for as many variables as you need with one try/catch while still tracking the name of all variables that cause a problem.

public class test {
    public static void main(String[] args) {
        String string1 = returnString("fdsa", "string1");
        String string2 = returnString("fbbbb", "string2");
    }

    private string returnString(String input, String varName) {
        String str = "";
        try {
            str = input;
            Integer.parseInt(str);
        } catch (NumberFormatException e) {
            System.out.println("Error processing " + varName);
        }
        return str;
    }
}
Luke
  • 838
  • 7
  • 17
2

I'm surprised nobody proposed something like this:

public class test {
public static void main(String[] args) {
    String errorContext;

    try {            
        String string1 = "fdsa";
        String string2 = "fbbbb";

        errorContext = "string1";
        Integer.parseInt(string1);

        errorContext = "string2";
        Integer.parseInt(string2);
    } catch (NumberFormatException e) {
        System.out.println(errorContext + " is wrong!")
        e.printStackTrace();
    }
    }
}

It is a very simple solution - simplistic, some would say - but it is also fairly clear and robust.

The point of this question - as I understand it - was not to provide a better way of converting strings to integers (even if it indeed may exist), but rather to find a way to say not only what went wrong in a long try block, but also where it went wrong. If one's goal is to display a meaningful error message to the user (ideally suggesting what to do rather than just complaining that something is wrong), then simply printing a stack trace is not sufficient, of course, and try-catching every line of code is not an attractive option either.

  • This would be useful for a few variables or a collection, but if you have many individual variables to check it would requires an additional line of code for each - greatly increasing the size of your try block with code that doesn't throw exceptions. – Daniel Siebert Jul 29 '16 at 14:13
0

You should avoid catching Runtime exceptions and use some other mechanism for identifying errors. In your case you could use matcher and your problem could be written as:

public class test {

    private static final Pattern pattern = Pattern.compile("\\d+");
    public static void main(String[] args) {
            String string1 = "fdsa";
            String string2 = "fbbbb";
            if (pattern.matcher(string1).matches()) {
                Integer.parseInt(string1);
            } else {
                //string1 is not an integer
            }
            ...
    }
}

Or you could just write

boolean errorInLineOne = !pattern.matcher(string1).matches();
...
Heisenberg
  • 3,153
  • 3
  • 27
  • 55
0

It's quite simplistic, but here's my solution..

public class test
{
    public static int tryParseInt(String str) throws Exception
    {
        try
        {
            return Integer.parseInt(str);
        }
        catch(Exception e)
        {
            System.err.println("tryParseInt: couldn't parse '"+str+"'");
            throw e;
        }
    }

    public static void main(String[] args)
    {
        try
        {
            String string1 = "fdsa";
            String string2 = "fbbbb";

            tryParseInt(string1);
            tryParseInt(string2);
        }
        catch (NumberFormatException e)
        {
            e.printStackTrace();
        }
    }
}
Khaled.K
  • 5,828
  • 1
  • 33
  • 51