Missing return statement help

[TheOnlyNemesis]

Hey, i am struggling with this program, i have tried everything i can think of but it still says missing return statement on public String processInput(String theInput){

package clientserverassignment;

import java.net.*;
import java.io.*;

public class Protocol1 {

    private static final int WAITING = 0;
    private static final int SENTTOS = 1;
    private static final int SENTMENU = 2;
    private static final int ANOTHER = 3;
    private int state = WAITING;
    private String[] resource = {"A Computer Program", "A Picture", "An E-book"};
    private String[] description = {"A program that says hello", "A humourous IT pic", "A book full of random stuff"};

    public String processInput(String theInput) {

        String theOutput = null;
        String menu = null;
        switch (state) {
            case WAITING: {
                theOutput = "Do you accept the TOS?";
                state = SENTTOS;
                return theOutput;
            }


            case SENTTOS: {
                if (theInput.equalsIgnoreCase("Yes")) {
                    menu = resource[0];
                    for (int i = 1; i < resource.length; i++) {
                        menu = menu + resource[i];
                    }
                    theOutput = menu;
                    state = SENTMENU;
                    return theOutput;
                } else {
                    if (theInput.equalsIgnoreCase("No")) {
                        theOutput = "Bye";
                        return theOutput;
                    } else {
                        theOutput = "Invalid choice. Do you accept the TOS?";
                        state = SENTTOS;
                        return theOutput;
                    }
                }
            }

            case SENTMENU: {
                int choice;
                try {
                    choice = Integer.parseInt(theOutput);
                    if (choice <= 3 && choice >= 1) {
                        theOutput = description[choice - 1];
                        state = SENTMENU;
                        return theOutput;
                    } else {
                        theOutput = "Enter a number from the list you fucktard" + menu;
                        state = SENTMENU;
                        return theOutput;
                    }
                } catch (NumberFormatException e) {
                    theOutput = "Wrong format for number, enter a valid choice " + menu;
                    state = SENTMENU;
                    return theOutput;

                }

            }

            case ANOTHER: {
                if (theInput.equalsIgnoreCase("Yes")) {
                    theOutput = menu;
                    state = SENTMENU;
                    return theOutput;
                } else {
                    if (theInput.equalsIgnoreCase("No")) {
                        theOutput = "Bye";
                        return theOutput;
                    } else {
                        theOutput = "Invalid choice. Do you accept the TOS?";
                        state = SENTTOS;
                        return theOutput;
                    }
                }
            }
        }
    }
}

[Momerath]

Here is the code block that is causing your problem:

public String processInput(String theInput) {

    String theOutput = null;
    String menu;
    switch (state) {
        case WAITING: {
            theOutput = "Do you accept the TOS?";
            state = SENTTOS;
            return theOutput;
        }

Notice when you format it properly that you are missing '}' characters.

[TheOnlyNemesis]

Hey, i just formatted and editted my first post, i have checked through the program and all closing braces match up with an opening.

[Momerath]

Since the compiler can't tell that the value of 'state' will only be one of the case values you have specified, it needs a default statement to handle the times when it is not. So add a default and a return for it.

If you followed the one entry/one exit philosophy, you wouldn't have this problem :)

[TheOnlyNemesis]

Hey, thanks for that, never heard of that philosophy but thanks as i do now.

[jon.kiparsky]

One exit is not something that I would hold up as an ideal - it's one way to get a thing done. The important thing is that a method's logic should be clear, and every path through it should be obvious. Often that means skipping out of the method when you know you're done, as you do in your switch statement, or it can mean setting a return variable and skipping to the end - whatever makes it easy to look at the thing and know what's going on without having to get a pen and paper and map it out.

I would suggest that you simplify your method by subbing out each case to a method, so your switch just says

case WAITING:  return waiting(param1, param2...);
case SENTTOS: return sentTOS(param1, param2...);
...

default: return defaultValue(param)

This makes your switch statement into a simple list of method calls. Now your switch is only one level of logic: if in this state, do X, if in that state, do Y, and so forth. If I need to know what X and Y are, I go to the methods and look. This also makes it easier to simplify each branch of the switch, which might be useful. You might be able to extract shared functionality, and simplify things even further. For example:

theOutput = "Do you accept the TOS?";
                state = SENTTOS;
                return theOutput;

appears over and over again, which says maybe it can be extracted to a method.
Looking at it, I wonder if it wouldn't make more sense to put your state strings into an array, indexed by state, so you don't have to return a String, you just look up the right string in your array, and life's even simpler. Oh, and you've also gathered a bunch of UI data - the customer-facing Strings - into one place in the program, which is a win as well.

What's also obvious, when this is simplified, is that every branch of the switch has a return (this is obscured when there's a lot of conditional code within a branch) so I know that once the default is there, I should get back from this switch without trouble, assuming nobody throws an exception down in the working methods.

You could also simplify this to

switch (state){
case WAITING:  returnVal= waiting(param1, param2...);
case SENTTOS: returnVal= sentTOS(param1, param2...);
...

default: returnVal= defaultValue(param)
)
return returnVal

which is precisely equivalent, and equally easy to read, so it's your choice. Both are easier than having working code in your cases.

One thing that you don't need is the curly braces on each case. When you go into a switch, you jump to the matching case and then run until you hit a break . Curly braces don't do anything there.

[JamesCherrill]

I wonder if it wouldn't make more sense to put your state strings into an array, indexed by state, so you don't have to return a String, you just look up the right string in your array, and life's even simpler.

Looks to me like a perfect time to use an enum.

[jon.kiparsky]

Yep. Might be, might be.
I had to stop somewhere, though. :)