Student/Class Project Java MySQL

Question

I have a Java application that uses a few switch statements to get user information about students and classes. I have three MySQL tables:

students

student_id | student_name | hometown

classes

class_id | classname | description

student_x_classes

student_id | student_name | class_id | classname

The code I have is below. Its works fine, but I must admit, is pretty ugly. I've been trying to simplify it and break down the big ugly methods to make it more readable but keep struggling. I'm very new to Java and MySQL, so your patience is appreciated!

package classselector;

import java.sql.*;
import java.util.Scanner;;

public class ClassSelectorApp {

  public static void main(String[] args) throws SQLException {

    int menuItem = -1;
    while (menuItem != 0) {
      menuItem = menu();
      switch (menuItem) {
        case 1:
          createStudent();
          break;
        case 2:
          signUp();
          break;
        case 3:
          listClasses();
          break;
        case 0:
          break;
        default:
          System.out.println("Invalid Input");
          break;

      }
    }
  }

  protected static int menu() {
    try {
      int choice;
      Scanner sc = new Scanner(System.in);
      System.out.println("\n Class Selection Menu");
      System.out.println("**********************************");
      System.out.println("0: Exit Menu");
      System.out.println("1: Create New Student");
      System.out.println("2: Sign Up For a Class");
      System.out.println("3: List Classes for All Students");
      System.out.println("**********************************");
      System.out.println("Enter a choice: ");
      choice = sc.nextInt();
      return choice;

    } catch (java.util.InputMismatchException e) {
      System.out.println("Invalid choice!");
    } catch (Exception e) {
      System.out.println("Something went wrong...");
    }
    return 0;
  }
  static void createStudent() {
    System.out.println("\nCreate Student\n");
    try {
      Scanner input = new Scanner(System.in);
      System.out.println("Enter a Student ID: ");
      String student_id = input.nextLine();
      System.out.println("Enter Student Name: ");
      String student_name = input.nextLine();
      System.out.println("Enter Student Hometown: ");
      String hometown = input.nextLine();
      Connection con = DriverManager.getConnection("jdbc:mysql://localhost:3306/ClassSelector?autoReconnect=true&useSSL=false", "root", "");
      String sql = "INSERT INTO students" + "(student_id, student_name, hometown)" + "VALUES (?, ?, ?)";
      PreparedStatement myStmt = con.prepareStatement(sql);
      myStmt.setString(1, student_id);
      myStmt.setString(2, student_name);
      myStmt.setString(3, hometown);
      myStmt.executeUpdate();
      System.out.println("New Student Added");

    } catch (SQLIntegrityConstraintViolationException ex) {
      System.out.println("This entry has duplicate student ID or Student Name, please try again");
    } catch (SQLException SQL) {
      SQL.printStackTrace();
    } catch (Exception exc) {
      exc.printStackTrace();
    }
  }

  static void signUp() {
    System.out.println("\nSign Up For a Class\n");
    try {
    Connection con = DriverManager.getConnection("jdbc:mysql://localhost:3306/ClassSelector?autoReconnect=true&useSSL=false", "root", "");
    Statement myStmt = con.createStatement();

      Scanner input = new Scanner(System.in);
      System.out.println("Enter Student ID: ");
      String user_entered_student_id = input.nextLine();



      ResultSet rs;
      rs = myStmt.executeQuery("SELECT student_name FROM ClassSelector.students WHERE student_id = " + user_entered_student_id);
      while (rs.next()) {
        String userEnterId = rs.getString("student_name");
        System.out.println("Is " + userEnterId + " the correct student? (Y/N)");
        String confirm = input.nextLine();

        if (confirm.equalsIgnoreCase("Y")) {
          ResultSet rs2 = myStmt.executeQuery("SELECT * FROM ClassSelector.classes");
          while (rs2.next()) {
            String avlClasses = rs2.getString("class_id") + "\t" + rs2.getString("classname") + "\t" + rs2.getString("description");
            System.out.println(avlClasses);
          }
        } else if (confirm.equalsIgnoreCase("N")) {
          System.out.println("Oops, let start over");
          return;
        }

        System.out.println("Enter Class ID from Classes Listed Above to Join: ");
        String selectedClass = input.nextLine();
        ResultSet rs3 = myStmt.executeQuery("SELECT * FROM ClassSelector.classes WHERE class_id = " + selectedClass);
        while (rs3.next()) {
          String innerJoin = (userEnterId + " has been added to " + rs3.getString("classname") + " " + rs3.getString("class_id"));
          System.out.println(innerJoin);
          String student_classJoin = "INSERT IGNORE INTO student_x_class" + "(student_id,student_name, class_id, classname)" + "VALUES (?, ?, ?, ?)";
          PreparedStatement pStmt = con.prepareStatement(student_classJoin);
          pStmt.setString(1, user_entered_student_id);
          pStmt.setString(2, userEnterId);
          pStmt.setString(3, rs3.getString("class_id"));
          pStmt.setString(4, rs3.getString("classname"));
          pStmt.executeUpdate();
          System.out.println("Would you like to enroll " + userEnterId + " into another class? (Y/N)");
          String additionalClass = input.nextLine();
          if(additionalClass.equalsIgnoreCase("Y")){
              signUp();
          }
          else{
              return;
          }
        }
      }
    } catch (java.sql.SQLException SQL) {
      SQL.printStackTrace();
    } catch (Exception EXC) {
      EXC.printStackTrace();
    }
  }

  static void listClasses() {
    System.out.println("\nStudent Enrollment\n");
    try {
      Scanner input = new Scanner(System.in);
      System.out.println("Enter Student ID to See What Classes they are enrolled in: ");
      String user_entered_student_id = input.nextLine();

      Connection con = DriverManager.getConnection("jdbc:mysql://localhost:3306/ClassSelector?autoReconnect=true&useSSL=false", "root", "");
      Statement myStmt = con.createStatement();

      ResultSet rs;
      boolean found = false;
      rs = myStmt.executeQuery("SELECT student_id, student_name, class_id, classname  FROM ClassSelector.student_x_class WHERE student_id = " + user_entered_student_id);
      while (rs.next()) {
        String studentInClass = (rs.getString("student_id") + "\t" + rs.getString("student_name") + "  " + rs.getString("class_id") + " " + rs.getString("classname"));
        if (user_entered_student_id.equals(rs.getString("student_id"))) {
          System.out.println(studentInClass);
          found = true;
        }
      }
      if (!found) {
        System.out.println("This Student does not Exist!");
      }

    } catch (java.sql.SQLException SQL) {
      SQL.printStackTrace();
    } catch (Exception EXC) {
      EXC.printStackTrace();
    }
  }
}

If you want to get a revision of this code reviewed, open another question. Major edits like the one I just rolled back have a tendency to invalidate answers. — nhgrif, 2 days ago

mdfst13 · Accepted Answer · 2016-03-06 18:41:00Z

Looping until exit requested

    int menuItem = -1;
    while (menuItem != 0) {
      menuItem = menu();
      switch (menuItem) {
        case 1:
          createStudent();
          break;
        case 2:
          signUp();
          break;
        case 3:
          listClasses();
          break;
        case 0:
          break;
        default:
          System.out.println("Invalid Input");
          break;

      }
    }

You can simplify this by using a do/while to ensure that you always run the first time.

    int menuItem;
    do {
      menuItem = menu();
      switch (menuItem) {
        case 1:
          createStudent();
          break;
        case 2:
          signUp();
          break;
        case 3:
          listClasses();
          break;
        case 0:
          break;
        default:
          System.out.println("Invalid Input");
      }
    } while (menuItem != 0);

I find this a bit clearer about what you are doing. But in this case, I think that I'd actually eliminate menuItem altogether.

    while (true) {
      switch (menu()) {
        case 0:
          return;
        case 1:
          createStudent();
          break;
        case 2:
          signUp();
          break;
        case 3:
          listClasses();
          break;
        default:
          System.out.println("Invalid Input");
      }
    }

This saves you a variable declaration and puts all the input processing in the switch. The original version actually processed the input twice in the case of a 0. Once in the switch and then again in the while. This will only ever process it once.

The return will have the effect of leaving the method. You do not need a separate break with the return.

I put the 0 case first, as that puts them in what I'd consider ascending numeric order. This will make no functional difference, so it's fine to do it whichever way makes you comfortable.

As a general rule, when I put the default case last, I do not put a break in it. This helps make it stand out from the others. Putting the break in does not hurt anything though. It's just unnecessary.

`try` with resources

    try {
      int choice;
      Scanner sc = new Scanner(System.in);

It would be more common in newer Java versions to use a try with resources here.

    try (Scanner sc = new Scanner(System.in)) {

Also, you don't need to declare your variables at the beginning of the block. Declare them when you are ready to initialize. The exception would be if you need them have greater scope than the block where they are initialized, but that's not true here. So

      choice = sc.nextInt();
      return choice;

could be

      int choice = sc.nextInt();
      return choice;

or even just

      return sc.nextInt();

Or create just once

Another issue is that you create and release a lot of scanners. Why not just do that once at the beginning of the program and keep it until you're done? If you make it a class field, then you can have it available to every method without constantly creating one.

public class ClassSelectorApp {

could be

public class ClassSelectorApp {

    static private Scanner sc = new Scanner(System.in);

Since this is outside any try block, it will crash the program if it fails. But you exit on failure anyway. Which makes sense, after all, what are you going to do if you can't read input?

The same thing may be true of the database connection, although there may be reasons why you should release that more often.

Thank you, this info is very valuable to me and I appreciate the time and effort from everyone. Very encouraging! — Rassisland, 2 days ago

tim · Answer 2 · 2016-03-06 18:21:02Z

Database Structure

Your database will contain a lot of duplication.

The whole point of a mapping table such as student_x_classes is that you can have m:n relations without having to duplicate any of the other data (so you save storage space, and avoid the problem of updating in multiple locations).

But as you include that data inside the table as well, that benefit is gone.

What you want to do is just have the ids in the mapping table.

Security

You sometimes use prepared statements, but not always. This isn't good, as it leads to SQL injection.

Whenever you have something like

"[...] WHERE student_id = " + user_entered_student_id

just use prepared statements to make it secure.

Structure

You have quite a lot of prints all over the place, which makes it a bit hard to follow your code. Your methods also do a lot of things. Ideally, you want that a method only does one thing.

For example:

createStudent reads input data, manages a connection, and writes to the database. What if you want to change your code so that input doesn't come from System.in anymore? You would need to rewrite your code all over the place. Same in case your db connection changes.

So it's best to rewrite this method: createStudent(Connection connection, int id, String name, String homeTown). Now, it only inserts the data into the database, and is easily reusable.

You can restructure the rest of your code in the same way.

If you also want to add additional classes, you could have a Student class which holds data such as the name and the id. Now, you could move your createStudent method there as well (or into a separate StudentDAO).

Formatting

2 spaces is not enough. The standard is 4 spaces, and that's really the minimum you want to keep your code readable. If your code is so deeply nested that you need less indentation, fix the nesting instead.
don't import *, but import the concrete classes you need instead.
use camelCase, not snake_case (and definitely do not mix the two styles, it's confusing).

Naming

don't shorten variable names, it makes your code harder to read.
your variable names could be more exact in general. Eg: input could be scanner (it's not input after all). sql could be insertStudentQuery, rs could be studentNames, rs2 could be classes, I'm not sure what avlClasses are, but there is likely a better name, etc.

Thanks, Tim, this really helps! One of the main problems I kept running into while trying to clean up this code was what variables to create outside of the main method. I tried to create the connection, statement, resultset, and scanner outside the main method, but was having trouble accessing them with the other methods below. — Rassisland, 2 days ago
Hi @Rassisland, you have enough reputation points now, so if you liked this post you can reward Tim with an upvote if you will ;-) — janos♦, 2 days ago
@Rassisland as your whole code is static right now, the public fields would also need to be static. Then, you can just access them like any other variable (alternatively, you could also create them in main and just pass them as arguments; I think this is the nicer approach, especially because you will have an easier time transforming that into OOP code once you get more familiar with Java). — tim, 2 days ago
@Rassisland If you go with static fields for now, be careful not to have too many fields. If something doesn't need to be a field, it's better to have it at a local level (a reader has to always think about the state of all fields across the multiple method calls, which can get complex quickly). I don't think that eg your statements or resultset need to be a field. If you do have duplicate queries, it's likely a better idea to extract whatever they are doing to a method. — tim, 2 days ago
@Rassisland just create what you need in main and pass it on. Although ideally you would want to create specific methods to get the input as well. so at the beginning of main you would have Connection con = ... and just pass than con everywhere else. move the scanner there as well. and then you read the input you need in the different cases and pass it on as well. — tim, 2 days ago

h.j.k. · Answer 3 · 2016-03-07 01:00:34Z

Connection pooling

For safe and efficient handling of JDBC connections, you should be closing them every time you are done with it, but it'll also be better if you let a connection pooling manager do it for you (they generally cache open connections for reusing and will close them when desired). I did a quick search online and HikariCP ranks high up on the search results, for example.

Tying database schema design in code

try {
    // ...
    String sql = "INSERT INTO students" + "(student_id, student_name, hometown)" 
                    + "VALUES (?, ?, ?)";
    PreparedStatement myStmt = con.prepareStatement(sql);
    myStmt.setString(1, student_id);
    myStmt.setString(2, student_name);
    myStmt.setString(3, hometown);
    myStmt.executeUpdate();
    System.out.println("New Student Added");
} catch (SQLIntegrityConstraintViolationException ex) {
    System.out.println(
                "This entry has duplicate student ID or Student Name, please try again");
}

While it's nice to provide a more concrete error message ("duplicate student ID or Student Name"), you are effectively hard-coding the message based on the schema design here, which may become outdated if you were to change schema. You may want to consider retrieving the exact error message from the Exception thrown here.

asked	2 days ago
viewed	620 times
active	2 days ago

current community

your communities

more stack exchange communities

Student/Class Project Java MySQL

`students`

`classes`

`student_x_classes`

3 Answers 3

Looping until exit requested

`try` with resources

Or create just once

Connection pooling

Tying database schema design in code

Your Answer

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

Linked

Hot Network Questions

current community

your communities

more stack exchange communities

Student/Class Project Java MySQL

students

classes

student_x_classes

3 Answers 3

Looping until exit requested

try with resources

Or create just once

Connection pooling

Tying database schema design in code

Your Answer

Sign up or log in

Post as a guest

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

Linked

Related

Hot Network Questions

`students`

`classes`

`student_x_classes`

`try` with resources