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

Address book in Java

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

Problem

I've been learning Java for a month now using programmingbydoing.com in addition with reading the Java reference. I've had very very basic programming experience and I'm trying to improve my skills. These assignments are my first large projects that I've attempted.

I attempted an address book twice. The first time I was unhappy with my code. It was extremely messy and confusing. The second time I used comments and managed to complete it, but it's definitely still awkward.

```
import java.util.Scanner;
class Book{
Scanner s = new Scanner(System.in);
//Nested class for each entry
class Entry{
private String first;
private String last;
private String address;
private String email;
Entry(String first, String last, String address, String email){
this.first = first;
this.last = last;
this.address = address;
this.email = email;
}
Entry(){
first = "";
last = "";
address = "";
email = "";
}
public void readEntry(){
System.out.println("First Name:"+first );
System.out.println("Last Name:"+last );
System.out.println("Address:"+address );
System.out.println("Email:"+email );
}
}

//Keeps track of how many entries are in the book
private int entries = 0;
Entry[] contents;
public void initEntries(int e){
contents = new Entry[e];
for (int i = 0;i0){
contents[entry] = new Entry();
for (int i = 0;i<entries-entry;i++){
if (entry+1==entries) contents[entry] = new Entry();
else{
Entry temp = contents[entry+i];
contents[entry+i] = contents[entry+i+1]; //Removes an entry end moves each entry after it one backwards.
contents[entry+i+1] = temp;
}
}
entries--;

Solution

Side note
The link you provided was to the wrong project, but they are right next to each other.

I scanned through the website that you are learning from, and the project Address book is found in the ArrayList section but there is no reference to the ArrayList type. https://docs.oracle.com/javase/8/docs/api/java/util/ArrayList.html "Java doc for ArrayList". Not to be harsh, but if I was the teacher I would be requesting a re-write.

Using inner classes can very rarely be useful. This is not one of those cases. Entry has a proper place in the project as you use it as a data model. Making an ArrayList in your Book class would be clean and to the point.

The Book class itself will become very easy and clean when you switch to ArrayList. For instance if you decided to use lambdas and kept using the number to sort your entries your code would look something like this in your switch

ArrayList list = new ArrayList<>();
//populate the entries
list.sort((entry1, entry2) -> entry1.last.compareTo(entry2.last));
//list is now sorted by last name


However passing in an integer to the sort method is not very clear. What I mean is that, without looking at your code can you tell me what number is needed to sort by for each property in Entry? Nor would I want to. Even if you were to put a good comment that described very clearly what each number represented, it's still not clean. No instead make sort take in a Comparator Then your code could look like one of two options.

book.sort((entry1, entry2) -> entry1.last.compareTo(entry2.last));

//or if you create a new class per property it could look like this
class sortEntriesByLastName : Comparator{...}
book.sort(new sortEntriesByLastName())


Enough on that. Next is why is Scanner s = new Scanner(System.in); in Book? it isn't used. Even if it was used, this would be a bad place for it. Keeping a clear separation of concerns (Single Repsonsibility Principle) will show that putting logic into classes and/or methods that makes it harder to debug and fix code.

Naming

public int getEntries() gets the count of entries, not the entries themselves. A better name for this method should be getEntriesCount or count or size. same goes with contents[entries]. I nit pick the word entries because you have a class called Entry. Most collections are either called the plural version of the class name, or by the logical name of a collection of classes. (For example a collection of Animals in a Person class could be called pets, instead of animals)

Be concerned when methods start to have more than 3 parameters. Your add method has 4, and the intent of it is to just add a new entry. So instead of passing in those 4 entries, just have Entry as a parameter.

Duplicate code: add and addFromCopy do the same thing. that being said create a new Entry with the 4 parameters and then call addFromCopy in add. Or better yet, just delete the method add, and use only addFromCopy (but rename it to add). Be mindful of when you have code that essentially does the same thing and when you spot it, pull it out into a shared method. (yes even if it is only for 2 or 3 lines of code)

Last but not least, is that I'm suprised that a new blog about learning how to program doesn't mention ANY THING about unit tests. I know it is geared for beginners learning the basics, but the basics can still be taught by using a testing framework instead of viewing the output on the console. Normally I'd give a brief example, and I still might, but I have to get going. If I have time tonight I will edit my post and show an example.

Code Snippets

ArrayList<Entry> list = new ArrayList<>();
//populate the entries
list.sort((entry1, entry2) -> entry1.last.compareTo(entry2.last));
//list is now sorted by last name
book.sort((entry1, entry2) -> entry1.last.compareTo(entry2.last));

//or if you create a new class per property it could look like this
class sortEntriesByLastName : Comparator<Entry>{...}
book.sort(new sortEntriesByLastName())

Context

StackExchange Code Review Q#98135, answer score: 3

Revisions (0)

No revisions yet.