3
\$\begingroup\$

I made a simple program with main class, Student class and several methods in main class doing job (list, create and Write to file, del item, add item, search and sort items). it's running good but, I wish to have it in more 'clean way' or 'object oriented' way, so to speak, e.g. to create another class Control where will be editing methods from main class. Below is the code. Thanks for any suggestions!

import java.util.ArrayList;
import java.util.Scanner;
import java.io.FileWriter;
import java.io.BufferedWriter;
import java.io.File;
import java.io.IOException;
import java.util.logging.Level;
import java.util.logging.Logger;

public class SimpleDB {

static FileWriter fstream;
static BufferedWriter out;
public static ArrayList<Student> students;
static Scanner menuSc;
static File file;
static Scanner nameOfFile;
static FileWriter wr;
static Scanner addNameAndEmail;
static Scanner sort;
static Scanner search;

public static void createWriteToFile() {
    try {

        String aktDir = System.getProperty("user.dir");
        System.out.println("Actual dir>" + aktDir);
        System.out.println("Please enter name of file ...\n");
        nameOfFile = new Scanner(System.in);
        String nof = nameOfFile.nextLine();

        file = new File(nof);
        if (!file.exists()) {
            System.out.println("Creating new file : " + nof);
        }
        try {    //create new file
            file.createNewFile();
        } catch (IOException ex) {
            Logger.getLogger(SimpleDB.class.getName()).log(Level.SEVERE, null, ex);
        }
        System.out.println("Writing to file " + nof);
        try {
            wr = new FileWriter(nof);
        } catch (IOException ex) {
            Logger.getLogger(SimpleDB.class.getName()).log(Level.SEVERE, null, ex);
        }
        for (Student stu : students) {
            try {
                wr.write(stu.toString());
            } catch (IOException ex) {
                Logger.getLogger(SimpleDB.class.getName()).log(Level.SEVERE, null, ex);
            }

        }
        wr.close();
    } catch (IOException ex) {
        Logger.getLogger(SimpleDB.class.getName()).log(Level.SEVERE, null, ex);
    }
}

public static void sort() {
    boolean b = true;
    while (true) {
        System.out.println("Sorting> by name: type N, by email: type E, for exit: type X \n");
        sort = new Scanner(System.in);
        String s = sort.nextLine();
        switch (s) {

            case "N":
                System.out.println("Sorting by name....: \n");
                students.sort((Student s1, Student s2) -> {
                    return s1.getName().compareToIgnoreCase(s2.getName());
                });

                System.out.println("Sorted by name> \n" + students);
                break;

            case "E":
                System.out.println("Sorting by mail....: \n");
                students.sort((Student s1, Student s2) -> {
                    return s1.getEmail().compareToIgnoreCase(s2.getEmail());
                });
                System.out.println("Sorted list> \n" + students);
                break;
            case "X":
                System.out.println("Returning to main menu...");
                b = false;
                return;

            default:
                System.out.println("Please enter correct choice! ");
                break;

        }

    }
}

public static void search() {
    System.out.println("Enter a name you want to search>  \n");
    search = new Scanner(System.in);
    boolean bol = false;
    String se = search.next();
    for (int i = 0; i <= students.size(); i++) {
        if (se.equalsIgnoreCase(students.get(i).getName())) {
            bol = true;
            break;
        }
    }
    if (bol) {
        System.out.println("found");
    } else {
        System.out.println("not found");

    }
}

private static void add() {
    addNameAndEmail = new Scanner(System.in);
    System.out.println("Please enter a name: ");
    String n = addNameAndEmail.nextLine();

    System.out.println("Please enter an email: ");
    String e = addNameAndEmail.nextLine();

    students.add(new Student(n, e));
    System.out.println("\n" + "new student " + students);
}

public static void list() {
System.out.println("List of Students> \n\n"+
students.toString().replaceAll("\\[|\\]",  "").replaceAll("," ,""));

    }

}

public static char menu() {
    System.out.println(""
            + " 'A' list, 'B' add, 'C' save to file, 'D' search, 'E' sort data, 'F' exit from program > ");
    menuSc = new Scanner(System.in);
    String c = menuSc.nextLine();
    if (c.isEmpty()) {
        return ' ';
    } //Files.copy(null, null, options) 
    return c.toUpperCase().charAt(0);
}

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

    // some students added
    students = new ArrayList<>();
    students.add(new Student("alan", "[email protected]"));
    students.add(new Student("michael", "[email protected]"));
    students.add(new Student("peter", "[email protected]"));
    students.add(new Student("andrew", "[email protected]"));
    boolean a = true;
    while (a = true) {

        char c = menu();

        switch (c) {

            case 'A':
                list();
                break;
            case 'B':
                add();
                break;
            case 'C':
                createWriteToFile();
                break;
            case 'E':
                sort();
                break;
            case 'D':
                search();
                break;
            case 'F':
                System.out.println("Good Bye!");
                a = false;
                return;

        }

    }

}
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I restructured quite a lot so I will just list the whole code and the things I changed.

First, I removed the static keyword everywhere, making all your static variables into members, and your static functions into member functions.

There were a lot of occasions you were using static variables (mostly Scanners) when you could have moved them to the scope of the function they were used in. Always try to keep variables in the smallest possible scope you can.

You had a small logical error in the following code. You assign = rather than compare == the value of a. Because you return in the 'F' case, this doesn't actually matter. I changed it to match your other while (true) loop.

boolean a = true;
while (a = true) {

You had a number of unclosed Scanners. Remember to close them when you're done with them. Moving these to the narrower scope made it easier for me to notice this.

I moved the setup from your main() function into the constructor of the SimpleDB class, and the code relating to "running" the database into a new function I named run.

You had a few poorly named variables such as:

boolean bol = false; // Renamed to isFound
String c = menuSc.nextLine(); // Renamed to choice

Try to think about what your variable represents and give it a meaningful name. Abbreviations such as your nof variable are bad. I renamed that to nameOfFile.

There were quite a few useless unused variables. Your IDE should warn you about this. Delete them if you no longer need them, they only clutter the code.

There are other improvements which could be made too. As a next step, I would try to break up long functions into smaller ones. I personally use a rule-of-thumb that anything longer than 10 lines could be split up. I would also look to improve the two while (true) loops - you were almost right with your implementation in your main() function.

public class SimpleDB
{
    private final ArrayList<Student> students;
    private final Scanner scanner;

    SimpleDB(Scanner scanner)
    {
        this.scanner = scanner;

        // some students added
        students = new ArrayList<>();
        students.add(new Student("alan",    "[email protected]"));
        students.add(new Student("michael", "[email protected]"));
        students.add(new Student("peter",   "[email protected]"));
        students.add(new Student("andrew",  "[email protected]"));
    }

    public void run()
    {
        boolean keepRunning = true;
        while (keepRunning)
        {
            switch (menu())
            {
                case 'A':
                    list();
                    break;
                case 'B':
                    add();
                    break;
                case 'C':
                    createWriteToFile();
                    break;
                case 'E':
                    sort();
                    break;
                case 'D':
                    search();
                    break;
                case 'F':
                    System.out.println("Good Bye!");
                    keepRunning = false;
            }
        }
    }

    private void createWriteToFile()
    {
        String aktDir = System.getProperty("user.dir");
        System.out.println("Actual dir>" + aktDir);
        System.out.println("Please enter name of file ...\n");

        String nameOfFile = scanner.nextLine();

        File file = new File(nameOfFile);
        if (!file.exists()) {
            System.out.println("Creating new file : " + nameOfFile);
        }
        try {    //create new file
            file.createNewFile();
        } catch (IOException ex) {
            Logger.getLogger(SimpleDB.class.getName()).log(Level.SEVERE, null, ex);
        }
        System.out.println("Writing to file " + nameOfFile);
        try (FileWriter wr = new FileWriter(nameOfFile))
        {
            for (Student stu : students) {
                wr.write(stu.toString());
            }
        } catch (IOException ex) {
            Logger.getLogger(SimpleDB.class.getName()).log(Level.SEVERE, null, ex);
        }
    }

    private void sort()
    {
        while (true) {
            System.out.println("Sorting> by name: type N, by email: type E, for exit: type X \n");
            String choice = scanner.nextLine();
            switch (choice) {

                case "N":
                    System.out.println("Sorting by name....: \n");
                    students.sort((Student s1, Student s2) -> {
                        return s1.getName().compareToIgnoreCase(s2.getName());
                    });

                    System.out.println("Sorted by name> \n" + students);
                    break;

                case "E":
                    System.out.println("Sorting by mail....: \n");
                    students.sort((Student s1, Student s2) -> {
                        return s1.getEmail().compareToIgnoreCase(s2.getEmail());
                    });
                    System.out.println("Sorted list> \n" + students);
                    break;
                case "X":
                    System.out.println("Returning to main menu...");
                    return;

                default:
                    System.out.println("Please enter correct choice! ");
                    break;

            }
        }
    }

    private void search() {
        System.out.println("Enter a name you want to search>  \n");

        boolean isFound = false;
        String query = scanner.nextLine();
        for (Student student : students) {
            if (query.equalsIgnoreCase(student.getName())) {
                isFound = true;
                break;
            }
        }
        if (isFound) {
            System.out.println("found");
        } else {
            System.out.println("not found");
        }
    }

    private void add()
    {
        System.out.println("Please enter a name: ");
        String name = scanner.nextLine();

        System.out.println("Please enter an email: ");
        String email = scanner.nextLine();

        students.add(new Student(name, email));
        System.out.println("\n" + "new student " + students);
    }

    private void list() {
        System.out.println("List of Students> ");
        for (Student stu : students) {
            System.out.println(stu);
        }
    }

    private char menu() {
        System.out.println(" 'A' list, 'B' add, 'C' save to file, 'D' search, 'E' sort data, 'F' exit from program > ");

        String choice = scanner.nextLine();

        if (choice.isEmpty())
        {
            return ' ';
        }

        return choice.toUpperCase().charAt(0);
    }

    public static void main(String[] args) throws IOException
    {
        Scanner scanner = new Scanner(System.in);
        SimpleDB db = new SimpleDB(scanner);
        db.run();
        scanner.close();
    }
}
\$\endgroup\$
8
  • \$\begingroup\$ appreciate the corrections..! i will try to do the improvements you suggested (esp. with long methods). I will then try to extend this db. by adding other classes(Teachers, management, Maintenance,..) . \$\endgroup\$ Commented Jan 6, 2017 at 9:31
  • \$\begingroup\$ i tried the code and it says there is some error...: Exception in thread "main" java.util.NoSuchElementException: No line found at java.util.Scanner.nextLine(Scanner.java:1540) at simpledb3.SimpleDB3.menu(SimpleDB3.java:163) at simpledb3.SimpleDB3.run(SimpleDB3.java:28) at simpledb3.SimpleDB3.main(SimpleDB3.java:177) Java Result: 1 Is there missing any exeption handling or so? \$\endgroup\$ Commented Jan 6, 2017 at 9:59
  • \$\begingroup\$ My mistake! I was using Scanner incorrectly. Sorry about that. I've updated my answer. Pay close attention to how I was able to reuse the same Scanner object throughout. Hope it helps. (I've actually tested it this time!) \$\endgroup\$
    – Michael
    Commented Jan 6, 2017 at 11:00
  • \$\begingroup\$ Ok, now it's running fine, thanks. I also tried to improve output in private void list() method, as to have it without brackets and commas, it works but it displays a bit wrong, there is a spacing from second line.. \$\endgroup\$ Commented Jan 6, 2017 at 15:04
  • \$\begingroup\$ Have you created a custom toString() method on your Student class? If you haven't, you should. \$\endgroup\$
    – Michael
    Commented Jan 6, 2017 at 15:13

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.