HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Simple telephone address book

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
telephonesimpleaddressbook

Problem

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()) {
Syst

Solution

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.

Code Snippets

String stringName = "";
String stringNumber = "";
System.out.println("Enter name: ");
stringName = input.nextLine();
System.out.println("Enter number: ");
stringNumber = input.nextLine();
System.out.println("Enter name: ");
String stringName = input.nextLine();
System.out.println("Enter number: ");
String stringNumber = input.nextLine();
stringCommand = input.nextLine();
String stringCommand = input.nextLine();
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");
    }

    ...

}

Context

StackExchange Code Review Q#26023, answer score: 3

Revisions (0)

No revisions yet.