Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have been staring at this code for awhile now and I am thinking there is a way to optimize it (namely the if-else statement with the for-loops). Any advice would be greatly appreciated.

Additionally, this needs to be done with an array list as that is my assignment requirement. I have all my code working, and it is not due for a few days, so I am looking for ways to improve it while extending my learning beyond the classroom.

The numberOfContacts variable is fed into the method from the main method, which gets the count from another method that reads through a text file to get the number of contacts.

/**
 * search method searches through the contact list for a given criteria
 * and displays all results.
 * @param searchString is type String.
 * @param type is type String.
 * @param numberOfContacts is type int.
 * @param contacts is type Person[].
 */
public static void search(String searchString, String type, int numberOfContacts, Person[] contacts) {
    // Initialize variables for results
    int found = 0;
    int[] results = new int[numberOfContacts];

    // Determine the type of search
    if (type.equals("name")) {
        // Search by name
        for (int x = 0; x < numberOfContacts; x++) {
            if (contacts[x].getName().contains(searchString)) {
                results[found] = x;
                found++;
            }
        }
    } else {
        // Search by phone
        for (int x = 0; x < numberOfContacts; x++) {
            if (contacts[x].getPhone().contains(searchString)) {
                results[found] = x;
                found++;
            }
        }
    }

    // Display the search results
    System.out.println("\n\t**************");
    System.out.println("\tSearch Results");
    System.out.println("\t**************");
    System.out.println("Found " + found + " results containing \"" + searchString + "\":");
    System.out.println();

    if (found > 0) {
        for (int x = 0; x < found; x++) {
            System.out.println(contacts[results[x]].getName() + "\t" + contacts[results[x]].getPhone());
        }
    }

    System.out.println("\n\n\n");
}
share|improve this question
add comment

2 Answers

1) You should clarify what your homework statement exactly says: a Java "array list" is something like List<Person> persons = new ArrayList<Person>(), while Person[] persons is a Java "array".

2) Java is not Fortran or C: there is no need for the argument int numberOfContacts, you can just call contacts.length to get the size of an array. If you use an ArrayList, it is contacts.size() instead.

3) When you print out the results, there is no need for if (found > 0) since the for-loop will not be executed a single time if there is nothing to execute.

Other than that the algorithm looks fine; I don't see much to optimize.
Optional suggestions:

4) Instead of using an if-statement to check the type, you can look into the Java switch-statement. That would be useful especially if you many types: "name", "phone", "address", "age", etc.

5) Instead of using hardcoded strings for the type, look into the enum:

public enum ContactField {
   NAME, PHONE;
}

and instead of passing an argument of type String, use the type ContactField. The switch-statement works with String and enum.

share|improve this answer
add comment

If you are limited to use only Arrays, you can create a interface like this

public static interface Checker {
    public boolean check(Person p, String searchString);
}

Create a Checker for every type of comparison you want.

And compare with the Checker

for (int x = 0; x < numberOfContacts; x++) {
    if (checker.check(contacts[x], searchString)) {
        results[found] = x;
        found++;
    }
}

You can see a working example here (with your classes and methos): http://ideone.com/2C6kei

Note: With this method is easy to expand the search criteria (as you see in the example, with a anywhere Checker).

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.