Java Simple Phonebook

Question

This Program is a simple phonebook, user is given 3 choice, add a number to the phone book, dial a number or exit the program. Are there Any Improvements that i can make, either efficient or cleaning the code or something useful i can add?

import java.util.ArrayList;
import java.util.Scanner;

public class PhoneBookV2 {
    public static void main(String[] args){
        final ArrayList<String> numbers = new ArrayList<>();
        print(numbers);
        task(numbers);
    }

    public static void task(ArrayList<String> numbers){
        Scanner scan = new Scanner(System.in);

        System.out.println("Please Pick A Task: \n 1:Add A Number to Speed Dial \n 2:Speed Dial A Number \n 3:Exit");
        String choice = scan.nextLine();

        switch(choice){
            case "1":

                AddNumber(numbers);
                break;
            case "2":
                CallNumber(numbers);
                break;
            case "3":
                System.out.println("Goodbye!");
                System.exit(0);
                break;
        }
    }

    private static void CallNumber(ArrayList<String> numbers) {
        Scanner scan = new Scanner(System.in);

        try {
            String[][] keys = {
                    {" ", "1", " ", "2", " ", "3"},
                    {" ", "4", " ", "5", " ", "6"},
                    {" ", "7", " ", "8", " ", "9"},
                    {" ", " ", " ", "0", " ", " "}
            };
            printPhoneBook(keys);

            System.out.println("Pick A Speed Dial Number");
            int choice = Integer.parseInt(scan.nextLine());

            String phoneNum = "";

            switch (choice) {
                case 1 -> phoneNum = numbers.get(0);
                case 2 -> phoneNum = numbers.get(1);
                case 3 -> phoneNum = numbers.get(2);
                case 4 -> phoneNum = numbers.get(3);
                case 5 -> phoneNum = numbers.get(4);
                case 6 -> phoneNum = numbers.get(5);
                case 7 -> phoneNum = numbers.get(6);
                case 8 -> phoneNum = numbers.get(7);
                case 9 -> phoneNum = numbers.get(8);
                case 0 -> phoneNum = numbers.get(9);
                default -> System.out.println("No Number Saved At " + choice);


            }
            if (phoneNum != null) System.out.println("Dialing " + phoneNum + "....");
            else System.out.println("There is No Number At This Location");
            PhoneBookV2.task(numbers);
        } catch(IndexOutOfBoundsException e) {
            System.out.println("There is No Number At This Location");
            PhoneBookV2.task(numbers);
        }
    }

    private static ArrayList<String> AddNumber(ArrayList<String> numbers) {
        Scanner scanner = new Scanner(System.in);
        boolean cont = false;

        do {
            System.out.print("Please Enter The Number You Wish To Save Under Speed Dial: ");

            // Add the next number to the ArrayList
            numbers.add(scanner.nextLine());

            System.out.print("Would you like to add another? Yes or No: ");
            String answer = scanner.nextLine().toLowerCase();
            if (answer.equals("yes")) continue;
            if (answer.equals("no")) {
                print(numbers);
                cont = true;
            }

        } while (!cont);
        PhoneBookV2.task(numbers);
        return numbers;
    }

    public static void printPhoneBook(String[][] keys){

        for(String[] row : keys){
            for(String s : row){
                System.out.print(s);
            }
            System.out.println();
        }
    }

    public static void print(ArrayList<String> numbers){
        for(int i = 0; i< numbers.size(); i++){
            System.out.println((i+1) + ") " + numbers.get(i));
        }

    }


}
```

Answer 1

These are my suggestions:

1. No need to invoke print before task in main. At that point, the numbers would be empty anyway.
2. Better to declare numbers as : List<String> numbers = new ArrayList<>(); (interface name on the left side) and make all parameters as List<String> numbers instead of ArrayList<String> numbers. This would minimize the code changes if you plan to use a different implementation of List instead of ArrayList in future.
3. You are currently not closing the Scanner objects. It would be good to use try with resources : try(Scanner s = new Scanner(System.in)) { //code}
4. Better to follow naming conventions : method names follow camel-case convention.
5. In CallNumber instead of the big switch-case, simple if-else is required. (Also it would be nice if speed dial 1 is numbers.get(1) instead of numbers.get(0), so that you could just use numbers.get(choice))

String phoneNum = null; // should not be initialized as "", otherwise dialing message would be displayed even if choice is invalid.
if(choice >= 0 && choice <= 9) {
    phoneNum = numbers.get((choice + 9) % 10);
} else {
    // error message
}

6. I believe cont in AddNumber is used to indicate continue adding numbers. But the loop executes when this is false. So the naming and the actual use do not match. You could have initialized it to true, then use a while(cont) instead of do-while(!cont) and for choice no you could have set cont = false.

Answer 2

It's hard to know where to begin here.

• If you only allow 10 numbers, why bother with an arraylist, an array would do
• If you allow a choice of 0 - 9, why do you have to change the chosen values for indexing in such an ugly way? If you want to use 1-based numbering for the phone numbers, just do that by simple arithmetic
• You use far too many Scanners, none of which you close - you only need one
• You use recursion - calling the "choose a task" method at the end of each task method - for a process which is naturally iterative. You've written loops elsewhere, so why not for the main process?
• You have no object-orientation - this is procedural code, just written in Java
• You don't validate input phone numbers - the user could enter -1 or "my dog has fleas" and the program would accept them
• As Gautham mentions (he makes a number of good points), the use of "cont" seems unintuitive
• The "if...else" in callNumber is non-standard format

Some basic ideas

• Split your code between the data and the user interaction - have a PhoneBook class with simple operations, like add, delete, retrieve and use that from your user interaction code
• Your main flow should then be a simple loop that asks for a choice of task, performs that task, then loops round again