Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

In addition to @tim@tim's answer...

static fields and methods

private static String playName;
private static String playDate;

The static modifier means that all LocalPlay objects will be sharing the same fields, which is likely not what you want. Furthermore, the getter and setter methods should not be static too: they should 'belong' to each instance.

try-with-resources

Your Scanner and BufferedWriter usage should be done through try-with-resources for safe and efficient handling of the underlying I/O resources:

public static void lPlayDetails() {
    try (Scanner scanner = new Scanner(System.in)) {
        // use scanner here, or pass it to other methods that require user inputs
    }
}

private static void addPlay(Scanner scanner) {
    System.out.print("Enter name of Play: ");
    String name = scanner.nextLine();
    // ...
    try (FileWriter fileWriter = new FileWriter("play.txt");
            BufferedWriter writer = new BufferedWriter(fileWriter)) {
        writer.write(name + " " + date);
    }
}

Input validation

System.out.print("Enter date of Play: ");
date = option.nextLine();

What happens if the user entered an empty String or a nonsensical input like "abc"?

Method calling

deletePlay() and viewPlay() should not be calling lPlayDetails(), having been called by that very method. They should just return - not literally with the return; statement, but implicitly at the end of the method.

In addition to @tim's answer...

static fields and methods

private static String playName;
private static String playDate;

The static modifier means that all LocalPlay objects will be sharing the same fields, which is likely not what you want. Furthermore, the getter and setter methods should not be static too: they should 'belong' to each instance.

try-with-resources

Your Scanner and BufferedWriter usage should be done through try-with-resources for safe and efficient handling of the underlying I/O resources:

public static void lPlayDetails() {
    try (Scanner scanner = new Scanner(System.in)) {
        // use scanner here, or pass it to other methods that require user inputs
    }
}

private static void addPlay(Scanner scanner) {
    System.out.print("Enter name of Play: ");
    String name = scanner.nextLine();
    // ...
    try (FileWriter fileWriter = new FileWriter("play.txt");
            BufferedWriter writer = new BufferedWriter(fileWriter)) {
        writer.write(name + " " + date);
    }
}

Input validation

System.out.print("Enter date of Play: ");
date = option.nextLine();

What happens if the user entered an empty String or a nonsensical input like "abc"?

Method calling

deletePlay() and viewPlay() should not be calling lPlayDetails(), having been called by that very method. They should just return - not literally with the return; statement, but implicitly at the end of the method.

In addition to @tim's answer...

static fields and methods

private static String playName;
private static String playDate;

The static modifier means that all LocalPlay objects will be sharing the same fields, which is likely not what you want. Furthermore, the getter and setter methods should not be static too: they should 'belong' to each instance.

try-with-resources

Your Scanner and BufferedWriter usage should be done through try-with-resources for safe and efficient handling of the underlying I/O resources:

public static void lPlayDetails() {
    try (Scanner scanner = new Scanner(System.in)) {
        // use scanner here, or pass it to other methods that require user inputs
    }
}

private static void addPlay(Scanner scanner) {
    System.out.print("Enter name of Play: ");
    String name = scanner.nextLine();
    // ...
    try (FileWriter fileWriter = new FileWriter("play.txt");
            BufferedWriter writer = new BufferedWriter(fileWriter)) {
        writer.write(name + " " + date);
    }
}

Input validation

System.out.print("Enter date of Play: ");
date = option.nextLine();

What happens if the user entered an empty String or a nonsensical input like "abc"?

Method calling

deletePlay() and viewPlay() should not be calling lPlayDetails(), having been called by that very method. They should just return - not literally with the return; statement, but implicitly at the end of the method.

Source Link
h.j.k.
  • 19.2k
  • 3
  • 36
  • 93

In addition to @tim's answer...

static fields and methods

private static String playName;
private static String playDate;

The static modifier means that all LocalPlay objects will be sharing the same fields, which is likely not what you want. Furthermore, the getter and setter methods should not be static too: they should 'belong' to each instance.

try-with-resources

Your Scanner and BufferedWriter usage should be done through try-with-resources for safe and efficient handling of the underlying I/O resources:

public static void lPlayDetails() {
    try (Scanner scanner = new Scanner(System.in)) {
        // use scanner here, or pass it to other methods that require user inputs
    }
}

private static void addPlay(Scanner scanner) {
    System.out.print("Enter name of Play: ");
    String name = scanner.nextLine();
    // ...
    try (FileWriter fileWriter = new FileWriter("play.txt");
            BufferedWriter writer = new BufferedWriter(fileWriter)) {
        writer.write(name + " " + date);
    }
}

Input validation

System.out.print("Enter date of Play: ");
date = option.nextLine();

What happens if the user entered an empty String or a nonsensical input like "abc"?

Method calling

deletePlay() and viewPlay() should not be calling lPlayDetails(), having been called by that very method. They should just return - not literally with the return; statement, but implicitly at the end of the method.