7
\$\begingroup\$

I'm new to programming, and I would appreciate any constructive criticism on my code and if there may be a more efficient way to code this.

import java.util.Scanner;
/**
 * This program will decide what mechanism a reaction will undergo given information about the reactants.
 * I will also include a mechanism to give a rudimentary explanation of the decision making process to
 * get the reaction mechanism.
 * I can make all of these Case Switches so I can account for misspellings and such
 */
public class mechanism
{
  public static void main(String[] args)
{
   System.out.println("Hello, this program is designed to figure out what mechanism a reaction will under go.");
   //The decision tree will be a series of if-else statements. If I find a better method, I will use that
   //I will add the Switch for the different types of reactions 

    Scanner keyboard = new Scanner(System.in);

    String Start = "y";
    String functional = "unassigned";//Defines if the functional roup is primary secondary or tertiary
    String Base = "unassigned";//Defines the strength of the base if needed
    String Polar = "unassigned";//Defines if the reactant is polarizable
    String Solvent = "unassigned"; //Defines if the solvent is protic or aprotic
    String Size = "unassigned";//Defines the Size of a Base
    String Heat = "unassigned";//This is for whether or not it is heated
    String Tetra = "unassigned";//Defines if a reaction can be tetra substituted
    String Alk = "unassigned";// Alkane or alkene
    int error = 0; //Defines error state

   while (Start.equalsIgnoreCase("y")) {
        System.out.println("Enter alkane to continue");
        Alk = keyboard.next();

        switch (Alk){
            case "alkane":System.out.println("Is the reactant soluble in the solvent? Answer in y or n.");
            String Solubility = keyboard.next(); //Defines if the reactant is soluble in the solvent

            switch(Solubility)
                             {

                  case "y":System.out.println("Is the functional group attached to a primary, secondary, or tertiary carbon?");
                    System.out.println(" Answer in p for primary, s for secondary, and t for tertiary.");

                    //keyboard = new Scanner(System.in); // Do not need to reinitialize Scanner
                    functional = keyboard.next();

                    switch (functional){
                        case "p":   System.out.println("All unimolecular reactions are ruled out, leaving E2 and Sn2.");
                        System.out.println("Is the reactant a strong base? Answer in y for yes or n for no");

                        //keyboard = new Scanner(System.in); //I do not need to reinitialize the scanner variable
                        Base = keyboard.next();
                            if (Base.equalsIgnoreCase("y") ){ //.equalsIgnoreCase works instead of == for strings. == compares memory addresses
                                System.out.println("The reaction undergoes E2 ");
                        }    else{
                                System.out.println("The reaction undergoes Sn2");
                            }                       

                        break;

                        case "s":System.out.println("No reactions have been ruled out.");
                                 System.out.println("Is the reactant a strong base? Answer in y or n");

                                 //keyboard = new Scanner(System.in);
                                 Base = keyboard.next();
                                 switch (Base) {
                                     case "y":System.out.println("Is it a bulky base or a skinny base?(b or s)");

                                              Size = keyboard.next();

                                            if (Size.equalsIgnoreCase("b")){
                                                System.out.println("This reaction undergoes E2");
                                            } else{
                                                System.out.println("Is the mixture being heated? y or n ");
                                                Heat = keyboard.next();
                                                if (Heat.equalsIgnoreCase("y")) {
                                                    System.out.println("This reaction undergoes E2");
                                                } else {
                                                    System.out.println(" This reaction undergoes Sn2");
                                                }
                                            }
                                              break;
                                     case "n":System.out.println("Is the reactant polarizable? y or n");
                                              Polar = keyboard.next();
                                              if (Polar.equalsIgnoreCase("y")) {
                                                  System.out.println("This reaction undergoes Sn2");
                                              } else {
                                                  System.out.println("Are the reactants being heated? y or n");
                                                  Heat = keyboard.next();
                                                  if(Heat.equalsIgnoreCase("y")){
                                                    System.out.println("This reaction undergoes E1");
                                                  } else{
                                                      System.out.println("Is the solvent aprotic or protic? Enter p or ap.");
                                                      Solvent = keyboard.next();
                                                      if (Solvent.equalsIgnoreCase("p")){
                                                          System.out.println("The reaction undergoes Sn1");
                                                      } else {
                                                          System.out.println("No reaction takes place");
                                                      }
                                                  }
                                              }
                                              break;
                                     default: error = 1;
                                    }                                    
                               break;

                         case "t": System.out.println("Sn2 has been ruled out");
                                System.out.println("Is it a strong base? y or n");
                                Base = keyboard.next();
                                switch (Base) {
                                    case "y": System.out.println("This reaction undergoes E2");
                                    break;

                                    case "n": System.out.println("Is the reaction being heated? y or n" );
                                              Heat = keyboard.next();
                                              System.out.println("If a carbocation can form, will it be able to re-arrange itself to form a tetra substituted product? y or n");
                                              Tetra = keyboard.next();

                                              if(Heat.equalsIgnoreCase("y") || Tetra.equalsIgnoreCase("y")){
                                                  System.out.println("This reaction undergoes E1");
                                              } else {
                                                  System.out.println("No reaction occurs");
                                              }
                                    break;

                                    default: error = 1;
                                }
                                break;

                         default: error = 1; 
                        }
                    break;

                  case "n":System.out.println("No reaction will occur.");
                    break;

                  default: error=1 ;
              } 
                break;

            default: error = 1 ;

        }

        if (error == 1) {
            System.out.println("Your response could not be understood");
            error = 0;
        }

        System.out.println("Would you like help on another reaction? y or n");
        Start = keyboard.next();
   }
}
}
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

First of all, you should pay attention to conventions such as capitalization and indentation. Class names should be UpperCase; variable names should be lowerCase. And for deeply nested code like this, consistently correct indentation is absolutely essential!


This decision tree appears to be an easy task. It turns out that doing it well is much more difficult than it seems. You'll see, from the suggested solution below, that a good solution isn't exactly obvious to beginners.

The problem with the naïve approach, whether using if/else or switch, is that it results in a lot of code repetition. Basically, the solution doesn't scale well if the decision tree grows. It's tricky to ensure that every menu behaves consistently, because it's copy-and-pasted code. Furthermore, it's difficult to do proper error handling.

You should, therefore, switch to a different strategy: data-directed programming. The behaviour should be primarily driven by a data structure — something that looks like this:

private static final Menu ALKANE_REACTION_DECISION_TREE = q("What kind of hydrocarbon?",
    a("alkane",
        q("Is the reactant soluble?",
            a("y",
                q("Is the functional group attached to a primary, secondary, or tertiary carbon?",
                    a("p",
                        q("All unimolecular reactions are ruled out, leaving E2 and Sn2.\n" +
                          "Is the reactant a strong base?",
                            a("y", result("The reaction undergoes E2")),
                            a("n", result("The reaction undergoes Sn2")))),
                    …

In a more elaborate program, this data structure might even be defined using , a file, or maybe even be stored in a database.


Suggested solution

Define supporting classes to represent the questions, answers, and results. I also suggest defining helper functions q(), a(), and result(), respectively, to avoid writing the new keyword everywhere when defining the tree.

Then, you can write a menu() loop that navigates that tree structure based on the user input.

import java.util.Arrays;
import java.util.Scanner;
import java.util.stream.Collectors;

public class ReactionMechanism {

    //////////////////////////////////////////////////////////////////////

    private interface MenuItem {
    }

    private static class Menu implements MenuItem {
        public final String question;
        public final Answer[] answers;
        public final String possibleAnswers;
        public Menu(String question, Answer... answers) {
            this.question = question;
            this.answers = answers;
            this.possibleAnswers = Arrays.stream(answers)
                                         .map(a -> "\"" + a.choice + "\"")
                                         .collect(Collectors.joining(" / "));
        }
    }

    private static class Answer {
        public final String choice;
        public final MenuItem decision;
        public Answer(String choice, MenuItem decision) {
            this.choice = choice;
            this.decision = decision;
        }
    }

    private static class Result implements MenuItem {
        public final String result;
        public Result(String result) {
            this.result = result;
        }

        public String toString() {
            return this.result;
        }
    }

    private static Menu q(String question, Answer... answers) {
        return new Menu(question, answers);
    }

    private static Answer a(String choice, MenuItem decision) {
        return new Answer(choice, decision);
    }

    private static Result result(String result) {
        return new Result(result);
    }

    //////////////////////////////////////////////////////////////////////

    private static final Menu ALKANE_REACTION_DECISION_TREE = q("What kind of hydrocarbon?",
        a("alkane",
            q("Is the reactant soluble?",
                a("y", // soluble
                    q("Is the functional group attached to a primary, secondary, or tertiary carbon?",
                        a("p", // primary
                            q("All unimolecular reactions are ruled out, leaving E2 and Sn2.\n" +
                              "Is the reactant a strong base?",
                                a("y", result("The reaction undergoes E2")),
                                a("n", result("The reaction undergoes Sn2")))),
                        a("s", // secondary
                            q("No reactions have been ruled out.\n" +
                              "Is the reactant a strong base?",
                                a("y", // strong base
                                    q("Is it a bulky base or a skinny base?",
                                        a("b", result("The reaction undergoes E2")),
                                        a("s", result("The reaction undergoes Sn2")))),
                                a("n", // not strong base
                                    q("Is the reactant polarizable?",
                                        a("y", result("The reaciton undergoes Sn2")),
                                        a("n",
                                            q("Are the reactants being heated?",
                                                a("y", result("The reaction undergoes E1")),
                                                a("n",
                                                    q("Is the solvent aprotic or protic?",
                                                        a("p",  result("The reaction undergoes Sn1")),
                                                        a("ap", result("No reaction takes place")))))))))),
                        a("t", // tertiary
                            q("Sn2 has been ruled out.\n" +
                              "Is it a strong base?",
                                a("y", result("The reaction undergoes E2")),
                                a("n", // not strong base
                                    q("Is the reaction being heated?",
                                        a("y", result("The reaction undergoes E1")),
                                        a("n", // unheated
                                            q("If a carbocation can form, will it be able to re-arrange itself to form a tetra substituted product?",
                                                a("y", result("The reaction undergoes E1")),
                                                a("n", result("No reaction occurs")))))))))),
                a("n", result("No reaction will occur"))))); // insoluble

    private static Result menu(Scanner scanner, Menu m) {
       do {
           System.out.println(m.question);
           System.out.print("Enter " + m.possibleAnswers + ": ");
           String input = scanner.next();
           for (Answer ans : m.answers) {
               if (ans.choice.equalsIgnoreCase(input)) {
                   if (ans.decision instanceof Result) {
                       return (Result)ans.decision;
                   } else {
                       return menu(scanner, (Menu)ans.decision);
                   }
               }
           }
           System.out.println("Invalid choice!");
       } while (true);
    }

    public static void main(String[] args) {
        try (Scanner scanner = new Scanner(System.in)) {
            Result result = menu(scanner, ALKANE_REACTION_DECISION_TREE);
            System.out.println(result);
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$
String Start = "y";
    String functional = "unassigned";//Defines if the functional roup is primary secondary or tertiary
    String Base = "unassigned";//Defines the strength of the base if needed
    String Polar = "unassigned";//Defines if the reactant is polarizable
    String Solvent = "unassigned"; //Defines if the solvent is protic or aprotic
    String Size = "unassigned";//Defines the Size of a Base
    String Heat = "unassigned";//This is for whether or not it is heated
    String Tetra = "unassigned";//Defines if a reaction can be tetra substituted
    String Alk = "unassigned";// Alkane or alkene

This look inappropriate. Consider using different types and looking into data type sizes. (the memory consumption here is larger than you would probably expect)

Your switch statement isn't significant enough to not be an if else statement. Consider switching to if else for sanity on your inner switch statement at the very least.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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