5
\$\begingroup\$

This is my first time working with Java exceptions. I have made a program that works to specifications, but I am wondering if there is anything I've done which is unconventional or not recommended. The application program does specify that there be line breaks between each message.

My driver is:

import java.util.Scanner;
public class ReadStrings {

    public static void main(String[] args) throws StringTooLongException{
        String str1;
        final int MAX_STRING_LENGTH = 20;
        Scanner input = new Scanner(System.in);
        System.out.println("Enter strings, enter DONE when finished:\n");
        str1 = input.nextLine();
        if(str1.equalsIgnoreCase("done"))
            System.exit(0); //exit on first prompt
        do{
            try
            {
                if(str1.length()>MAX_STRING_LENGTH)
                    throw new StringTooLongException();
                System.out.println("\nEnter a string, enter DONE when finished:\n");
                str1 = input.nextLine();            
            }
                catch(StringTooLongException e)
            {
                    System.out.println(e.getMessage());
                    str1 = input.nextLine();
            }
        }while(!str1.equalsIgnoreCase("done"));
        input.close();
    }
}

And my custom exception class is:

public class StringTooLongException extends Exception {
    public StringTooLongException()
    {
        super("\nString has too many characters\n\nPlease try again:\n");       
    }
}

Thanks for taking a look. Any advice is appreciated.

\$\endgroup\$
4
\$\begingroup\$
if(str1.equalsIgnoreCase("done"))
        System.exit(0); //exit on first startup

Do not use System.exit() (somewhere else then in a catch block in main) It terminates the JVM immediately. This is usually not intended.
just write a return instead.

         if(str1.length()>MAX_STRING_LENGTH)
                throw new StringTooLongException();
            System.out.println("\nEnter a string, enter DONE when finished:\n");
            str1 = input.nextLine();         

This will result in an endless loop when the user enters a long string. The check is done (and the exception thrown) before she gets a chance to enter a new one.

You should move the if behind the input request and delete the initial input request (along wit the if there) before the loop.


Since you do nothing else in the loop the checking the string and requesting new one some may argue that you use the Exception as flow control for which exceptions are not supposed to be used.
I'd agree to that argument in this particular case as long as the "happy path" also repeats the loop...

\$\endgroup\$
  • 1
    \$\begingroup\$ Another reason why it's better to use return instead of System.exit: System.exit prevents the finalizers in the main method (and other parts of the stack) from being called, resulting in a less than clean cleanup \$\endgroup\$ – Ferrybig Nov 25 '16 at 13:02
2
\$\begingroup\$
        String str1;

Delete that and change

        str1 = input.nextLine();

to

        String str1 = input.nextLine();

instead.

        if(str1.equalsIgnoreCase("done"))
            System.exit(0); //exit on first prompt
        do{
            try
            {
                if(str1.length()>MAX_STRING_LENGTH)
                    throw new StringTooLongException();
                System.out.println("\nEnter a string, enter DONE when finished:\n");
                str1 = input.nextLine();            
            }
                catch(StringTooLongException e)
            {
                    System.out.println(e.getMessage());
                    str1 = input.nextLine();
            }
        }while(!str1.equalsIgnoreCase("done"));

This seems the long way around. Why not

        while (!str1.equalsIgnoreCase("done")) {
            if (str1.length() > MAX_STRING_LENGTH)
            {
                System.out.println("\nString has too many characters\n\nPlease try again:");
            }

            System.out.println("\nEnter a string, enter DONE when finished:\n");
            str1 = input.nextLine();
        }

It's uncommon to throw an exception in a try block that you catch immediately. And it's totally unnecessary in this case. The only thing that you do differently is to print a message.

While not unknown, it is uncommon to put a try/catch in a loop. It's extra overhead that you don't need in this case.

It's unclear why you exit before the loop. You can just use a regular while loop and exit normally. You also close input in that case.

\$\endgroup\$
  • \$\begingroup\$ It was unclear to me too. After I corrected my mistake, moving the if statement behind the input request, I no longer had the problem of being unable to enter done to exit the loop on the first execution. The exercise actually required a try/catch and throw to be used. I agree though, the if statement is much easier for me to understand. \$\endgroup\$ – RandomKaryotype Nov 24 '16 at 21:37
1
\$\begingroup\$

You could make StringTooLongException a subclass of RuntimeException instead of Exception to make it an unchecked exception, in the spirit similar to the unchecked IndexOutOfBoundsException and ArithmeticException. However, this depends on the API you are willing to present to your users, and whether you think that this exception must be handled specially over others.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.