Search code examples
javaswitch-statementnested-if

Would it be easier to keep code as nested if statements or to try and convert to a switch or multiple switches


I finished a coding assignment for class and was given feedback by a friend that the nested ifs might look a lot better as switch statements

I tried converting it into switches but it seems to be about the same if not more work.

//this is the absolute basic frame of the code created
if(arg){
    //code
    if(arg){
        //code
        if(arg){
            //code
        }
        else if(arg){
            //code
        }
    }
    else if(arg){
        //code
    }
else if(arg){
    //code
}

Would it be easier and better looking if it was converted to switch statements or would it be the same if not worse mess?

EDIT: for ghostcat, this is the full section of code that I simplified for the question

while(loop == true){
        System.out.print("Do you have more students to enter (Y for yes, N for no): ");
        yn = input.nextLine();
        if(yn.equals("Y") || yn.equals("y")) {
            System.out.print("Undergraduate or Graduate? (U for undergraduate, G for graduate): ");
            ug = input.nextLine();
            if(ug.equals("U") || ug.equals("u")) {
                System.out.print("Student name: ");
                NAME = input.nextLine();
                System.out.print("Student ID: ");
                ID = input.nextInt();
                System.out.print("Student GPA: ");
                GPA = input.nextFloat();
                System.out.print("Is student a transfer student? (Y for yes, N for no): ");
                input.nextLine();
                transfer = input.nextLine();
                if(transfer.equals("Y") || transfer.equals("y")) {
                    ts = true;
                    students.add(new UndergradStudent(NAME, ID, GPA, ts));
                }
                else if(transfer.equals("N") || transfer.equals("n")) {
                    ts = false;
                    students.add(new UndergradStudent(NAME, ID, GPA, ts));
                }
            }
            else if(ug.equals("G") || ug.equals("g")) {
                System.out.print("Student name: ");
                NAME = input.nextLine();
                System.out.print("Student ID: ");
                ID = input.nextInt();
                System.out.print("Student GPA: ");
                GPA = input.nextFloat();
                System.out.print("What college did the student graduate from: ");
                input.nextLine();
                college = input.nextLine();
                students.add(new GradStudent(NAME, ID, GPA, college));
            }
        }
        else if(yn.equals("N") || yn.equals("n")) {
            loop = false;
        }
    }

Solution

  • From the clean code perspective, both options (if/else or switches) aren't preferable. But solving that without more context isn't possible.

    First of all, the real problem is that your method that sits around your example code has to look at so many parameters.

    The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification—and then shouldn’t be used anyway.

    In other words: you strive to write methods that have as few arguments as possible. Because each argument potentially adds the need for such an if or switch contrast.

    When several switch statement would work for you, one potential path might be turn to polymorphism. The "correct" way of switching in OOP is to have different classes, and use runtime polymorphism to determine which method actually gets invoked at runtime.

    Given the added context:

    ug = input.nextLine();
    if(ug.equals("U") || ug.equals("u")) {
    ...
    

    The clean code solution could go like this:

    ug = input.nextLine();
    if (ug.equalsIgnoreCase("u")) {
      fetchValuesForUndergraduate(); 
    }
    

    That is it! Your real problem with your current code is that it does so many things in one spot. The "clean code" solution to that is to move code into distinct helper methods, that have nice telling names, and that do much less.