Simple telephone address book

Question

I am new to Java programming, and to help me learn, I created a simple telephone address book. The code runs as expected, but I would be interested to here from more advanced programmers if they think I should do it differently.

import com.mongodb.MongoClient;
import com.mongodb.MongoException;
import com.mongodb.DB;
import com.mongodb.DBCollection;
import com.mongodb.BasicDBObject;
import com.mongodb.DBCursor;

import java.net.UnknownHostException;
import java.util.Scanner;

import org.bson.types.ObjectId;

public class MongodbPhonebook {
    public static void main(String[] args) {
        boolean boolContinue = true;
        String stringCommand;
        Scanner input = new Scanner(System.in);

        try {
            MongoClient mongoClient = new MongoClient();
            DB db = mongoClient.getDB( "myphonelist" );
            DBCollection coll = db.getCollection("myphonelist");



            do {
                System.out.println("Enter command (add, find, show, quit): ");
                stringCommand = input.nextLine();
                if(stringCommand.equals("quit")){
                    boolContinue = false;
                }
                else if(stringCommand.equals("show")){
                    displayAll(coll);
                }
                else if(stringCommand.equals("add")){
                    insertNumber(coll);
                }
                else if(stringCommand.equals("find")){
                    findNumber(coll);
                }
                else {
                    System.out.println("Unknown command!");
                }
            } while(boolContinue == true);
        }
        catch (UnknownHostException e) {
            e.printStackTrace();
        }
        catch (MongoException e) {
            e.printStackTrace();
        } 
    }

    public static void displayAll(DBCollection collDisplay) {
        DBCursor cursor = collDisplay.find();
        try {
            while(cursor.hasNext()) {
                System.out.println(cursor.next());
            }
        } finally {
            cursor.close();
        }    
        return;
    }

    public static void insertNumber (DBCollection collInsert) {
        Scanner input = new Scanner(System.in);
        ObjectId stringID;
        String stringName = "";
        String stringNumber = "";
        System.out.println("Enter name: ");
        stringName = input.nextLine();
        System.out.println("Enter number: ");
        stringNumber = input.nextLine();
        BasicDBObject doc = new BasicDBObject("name", stringName).
                append("number", stringNumber);
        collInsert.insert(doc);
        stringID = (ObjectId)doc.get( "_id" );
        System.out.println("Result: " + stringID.toString());
        return;
    }

    public static void findNumber (DBCollection collFind) {
        Scanner input = new Scanner(System.in);
        String stringName = "";
        System.out.println("Enter name: ");
        stringName = input.nextLine();
        BasicDBObject query = new BasicDBObject("name", stringName);

        DBCursor cursor = collFind.find();
        cursor = collFind.find(query);

        try {
            while(cursor.hasNext()) {
                System.out.println("found " + cursor.next());
            }
        } finally {
            cursor.close();
        }
    }
}

Answer 1

Overall, it looks pretty good. Here are some points to consider:

There is no need to explicitly return at the end of void methods, for example, at the end of displayAll().

Keep your variable declarations close to where they are used. For example, in insertNumber(), I would change this...

String stringName = "";
String stringNumber = "";
System.out.println("Enter name: ");
stringName = input.nextLine();
System.out.println("Enter number: ");
stringNumber = input.nextLine();

...to this...

System.out.println("Enter name: ");
String stringName = input.nextLine();
System.out.println("Enter number: ");
String stringNumber = input.nextLine();

Also, limit the scope of variables to the block where they are used. This follows from the point above. For example, in main(), stringCommand is declared near the top, but is only used inside the do/while block. I would remove the declaration above and change this line...

stringCommand = input.nextLine();

...to this...

String stringCommand = input.nextLine();

These will reduce the number of lines slightly, but will also make reading and debugging the code easier.

As a slighter bigger project, you could change the methods to be non-static, then create an instance of MongodbPhonebook in main() to interact with instead. It may also make sense to move some of the code from main() somewhere else. For example, you could make the mongo variables private members of MongodbPhonebook and initialize them in the constructor.

public class MongodbPhonebook {

    private MongoClient mongoClient;
    private DB db;
    private DBCollection coll;

    public MongodbPhonebook() {
        this.mongoClient = new MongoClient();
        this.db = mongoClient.getDB( "myphonelist" );
        this.coll = db.getCollection("myphonelist");
    }

    ...

}

If you did this, you would no longer need to pass the DBCollection around to the other functions.

Beyond that, consider separating the user interaction and database logic into separate functions. For example, split findNumber() into two functions: one to ask the user for input and display the results, the other to perform the search based on the supplied input. This would allow the database logic functions to be reused in different circumstances, for example, a web application.

Answer 2

I'm not sure what version of the JDK you are using, but in Java 7 they introduced switch statements on a String variable instead of just integers. Your code could instead read like this, to make it a little bit more readable.

do {
                System.out.println("Enter command (add, find, show, quit): ");
                stringCommand = input.nextLine();
                switch(stringCommand)
                {
                    case "quit":
                         boolContinue = false;
                         break;
                    case "show":
                         displayAll(coll);
                         break;
                    case "add":
                         insertNumber(coll);
                         break;
                    case "find":
                         findNumber(coll);
                         break;
                    default:
                         System.out.println("Unknown command!");
                         break;
                }
            } while(boolContinue);   // A boolean variable by itself can be used as an expression. The == true expression is redundant.