patternjavaModerate
Student record system
Viewed 0 times
recordstudentsystem
Problem
In particular, I'm not happy with case 5 in the Control class. Basically, in case 5, I would like the user to attach a student from the students array onto a module from the modules array without duplication. Any suggestions/example would be very useful.
Driver Class:
Control class:
```
import java.util.Scanner;
public class Control {
// Integers represented as module codes
private static final int UFCE1 = 0;
private static final int UFCE2 = 1;
private static final int UFCE3 = 2;
private static final int UFCE4 = 3;
private static final int UFCE5 = 4;
// Integers represented as students
private static final int STUDENT1 = 0;
private static final int STUDENT2 = 1;
private static final int STUDENT3 = 2;
private static final int STUDENT4 = 3;
private static final int STUDENT5 = 4;
public void run() {
Student jane = new Student("jane");
Student alex = new Student("alex");
Student mike = new Student("mike");
Student james = new Student("james");
Student julia = new Student("julia");
Student[] students = new Student[]{jane, alex, mike, james, julia};
Module ufce1 = new Module("UFCE1");
Module ufce2 = new Module("UFCE2");
Module ufce3 = new Module("UFCE3");
Module ufce4 = new Module("UFCE4");
Module ufce5 = new Module("UFCE5");
Module[] modules = new Module[]{ufce1, ufce2, ufce3, ufce4, ufce5};
jane.addModule(ufce1.getName());
jane.addModule(ufce3.getName());
alex.addModule(ufce1.getName());
alex.addModule(ufce2.getName());
ufce1.addStudent(jane.getName());
ufce3.addStudent(jane.getName());
ufce1.addStudent(alex.getName());
ufce2.addStudent(alex.getName());
System.out.println("Welcome to Student Record System!");
while (true) {
Menu menu = new Menu();
menu.getMainMenu();
try {
Scanner scan = new S
Driver Class:
public class MyApplication {
public static void main(String[] args) {
Control control = new Control();
control.run();
}
}Control class:
```
import java.util.Scanner;
public class Control {
// Integers represented as module codes
private static final int UFCE1 = 0;
private static final int UFCE2 = 1;
private static final int UFCE3 = 2;
private static final int UFCE4 = 3;
private static final int UFCE5 = 4;
// Integers represented as students
private static final int STUDENT1 = 0;
private static final int STUDENT2 = 1;
private static final int STUDENT3 = 2;
private static final int STUDENT4 = 3;
private static final int STUDENT5 = 4;
public void run() {
Student jane = new Student("jane");
Student alex = new Student("alex");
Student mike = new Student("mike");
Student james = new Student("james");
Student julia = new Student("julia");
Student[] students = new Student[]{jane, alex, mike, james, julia};
Module ufce1 = new Module("UFCE1");
Module ufce2 = new Module("UFCE2");
Module ufce3 = new Module("UFCE3");
Module ufce4 = new Module("UFCE4");
Module ufce5 = new Module("UFCE5");
Module[] modules = new Module[]{ufce1, ufce2, ufce3, ufce4, ufce5};
jane.addModule(ufce1.getName());
jane.addModule(ufce3.getName());
alex.addModule(ufce1.getName());
alex.addModule(ufce2.getName());
ufce1.addStudent(jane.getName());
ufce3.addStudent(jane.getName());
ufce1.addStudent(alex.getName());
ufce2.addStudent(alex.getName());
System.out.println("Welcome to Student Record System!");
while (true) {
Menu menu = new Menu();
menu.getMainMenu();
try {
Scanner scan = new S
Solution
- There doesn't seem to be any reason for where you've split your Menu and Controller classes. Most of the menu logic is still in controller.
- If you ever find yourself defining variables or constants with sequential numbers like: STUDENT1, STUDENT2, STUDENT3, you are doing it wrong.
- Your run method is way too long. Move the code for each menu item to its own function and call them.
- You've got one try/catch block around the whole function. This is a bad idea as it will catch the wrong exceptions. You should only have the actual call that can fail inside the try block
- Your try catch block catches any kind of exception. This is a bad idea since it may hide bugs when something other then the expected function fails.
- I don't understand why you wrote case 5 as you did. Every case inside your switches is the same except for the number you are using to index. And you already have that number inside a variable. Just use that variable as the index and toss the switch/case.
Context
StackExchange Code Review Q#6454, answer score: 12
Revisions (0)
No revisions yet.