Validating data to nest if or not within try and catch

Question

I am validating data, in this case I want one of three ints. I am asking this question, as it is the fundamental principle I'm interested in. This is a basic example, but I am developing best practices now, so when things become more complicated later, I am better equipped to manage them.

Is it preferable to have the try and catch followed by the condition:

public static int getProcType()
{
    try
    {
        procType = getIntInput("Enter procedure type -\n"
            + " 1 for Exploratory,\n" 
            + " 2 for Reconstructive, \n"
            + "3 for Follow up: \n");
    }
    catch (NumberFormatException ex)
    {
        System.out.println("Error! Enter a valid option!");
        getProcType();
    }
    if (procType == 1 || procType == 2 || procType == 3)
    {
        hrlyRate = hrlyRate(procType);
        procedure = procedure(procType);
    }
    else
    {
        System.out.println("Error! Enter a valid option!");
        getProcType();
    }

    return procType;
}

Or is it better to put the if within the try and catch?

public static int getProcType()
{
    try
    {
        procType = getIntInput("Enter procedure type -\n"
            + " 1 for Exploratory,\n" 
            + " 2 for Reconstructive, \n"
            + "3 for Follow up: \n");

        if (procType == 1 || procType == 2 || procType == 3)
        {
            hrlyRate = hrlyRate(procType);
            procedure = procedure(procType);
        }
        else
        {
            System.out.println("Error! Enter a valid option!");
            getProcType();
        }
    }
    catch (NumberFormatException ex)
    {
        System.out.println("Error! Enter a valid option!");
        getProcType();
    }

    return procType;
}

I am thinking the if within the try, may be quicker, but also may be clumsy. Which would be better, as my programming becomes more advanced?

_both_ ways are bad, as this code mixes two very different things into one method (handling user input and doing business logic) - this is explained in details in answers to duplicate question: [Should I extract specific functionality into a function and why?](http://programmers.stackexchange.com/questions/166884/should-i-extract-specific-functionality-into-a-function-and-why) — gnat, Oct 29 '13 at 11:53
@Skippy: have you had the time to check out [codereview.se]? You might find it useful... But yes, try to always include some information on what you're hoping to accomplish - it makes identifying and evaluating the question much easier for others. Small amounts of irrelevant info can always be removed later. — Shog9, Oct 29 '13 at 16:25

score 0 · Accepted Answer · answered Oct 29 '13 at 14:36

I wouldn't bother using an exception. Instead just a simple test on an IF condition on the method/function.

As @gnat says, putting this in a seprate merthod/function is the best. Also, consider adding an enumeration for the return with an extra value like:

 Exploratory,
 Reconstructive, 
 FollowUp, 
 Undefined

So, when getProcType() is called it can return "Undefined" back. If your program gets that display a message to the user and then quit. If it gets a proper value, it continues and calls another method to call the hourly rate and procedure (separate method).

In your code, how does the user cancel out? They could get stuck trying to enter 1,2,3 as the function keeps calling itself. It usually better to fail fast. So, politely tell the user they entered the wrong value, display a list of acceptable values and then quit. I'm assuming this is some sort of command line utility.

Also, I can suggest using a more sophisticated UI like a list box/combo box which would eliminate having the user typing anything. Then this logic would not be needed at all. In general we want to avoid having the using type in things whenever possible.

Thank you. It's for a uni assignment.. so the UI is erm.. no UI lol — , Oct 29 '13 at 15:01
procType is being called from another method, which controls the flow with an option to exit.. It quits that section of code when it gets a 1, 2, or 3. ts for your help, I'll be back with the next question no doubt :)\ — , Oct 29 '13 at 16:17

Validating data to nest if or not within try and catch

1 Answers1