Java problem

Started by
4 comments, last by Zahlman 17 years ago
For one of the exercises of the java tutorials im following i had to make a simple phonebook. Now i did that, but i thought about making it a lil better with things like searching for first name, last name or both. The implementation of the first name and both works, but i dont know how to do the last and show all the people with the same last name. Now of course you can implement it in a lot of ways, but i did it with the template of the exercise, so this is the code with a comment where the last name thing has to happen: PhoneEntry class:
class PhoneEntry {
  private String firstName; //first name of a person
  private String lastName; //last name of a person
  private String phone; //their phone number
  
  //constructor:
  public PhoneEntry(String fn, String ln, String p) {
    firstName = fn;
    lastName = ln;
    phone = p;
  }

  public String getFirstName() {
    return firstName;
  }
  
  public String getLastName() {
    return lastName;
  }  

  public String getPhone() {
    return phone;
  }
}  


PhoneBook class:
class PhoneBook
{ 
  PhoneEntry[] phoneBook; 

  PhoneBook()    // constructor
  {
    phoneBook = new PhoneEntry[ 5 ] ;

    phoneBook[0] = new PhoneEntry( "James", "Barclay", "(418) 665-1223" );
    phoneBook[1] = new PhoneEntry( "Grace", "Smith", "(860) 399-3044" );
    phoneBook[2] = new PhoneEntry( "Paul", "Smith", "(815) 439-9271" );
    phoneBook[3] = new PhoneEntry( "Violet", "Smith", "(312) 223-1937" );
    phoneBook[4] = new PhoneEntry( "John", "Wood", "(913) 883-2874" );

  }

  PhoneEntry search( String targetName1, String targetName2 )
  {
    // use linear search to find the targetName
    for (int j = 0; j < phoneBook.length; j++) {
      if(phoneBook[j].getFirstName().toUpperCase().equals(targetName1.toUpperCase()) && phoneBook[j].getLastName()
         .toUpperCase().equals(targetName2.toUpperCase())) {
        return phoneBook[j];
      }
      else if (phoneBook[j].getFirstName().toUpperCase().equals(targetName1.toUpperCase())) 
        return phoneBook[j];
    
 //here the it has to return the elements which contain persons with the given last name
 // else if (phoneBook[j].getLastName().toUpperCase().equals(targetName2.toUpperCase()))
    //    return phoneBook[j];
    }
    return null;
  }
}



PhoneBookTester class:
import java.util.Scanner;

class PhoneBookTester
{
 public static void main (String[] args)
 {
   PhoneBook pb = new PhoneBook();
   Scanner scan = new Scanner(System.in);
   PhoneEntry entry;

   System.out.print("Last Name? ");
   String lastname = scan.nextLine().toUpperCase();
   
   System.out.print("First Name? ");                      
   String firstname = scan.nextLine().toUpperCase();
   
   while(!lastname.equals("quit".toUpperCase())) {
     // search for name
     entry = pb.search( firstname, lastname );

     if ( entry != null )
       System.out.println( entry.getFirstName() + " " + entry.getLastName() + ": " + entry.getPhone() );
     else
       System.out.println("Name not found");
       
     System.out.print("\nLast Name? ");
     lastname = scan.nextLine().toUpperCase();
   
     System.out.print("First Name? ");
     firstname = scan.nextLine().toUpperCase();
   }
   System.out.println("Goodbye");

 }
}



Normal output will be something like this: Last Name? Smith First Name? Violet Violet Smith: (312) 223-1937 but i want something like this too, but dont know how to do it: Last Name? Smith First Name? John Smith: (812) 339-4916 Violet Smith: (312) 223-1937 Willoughby Smith (312) 992-8761 I hope someone can help me with this [Edited by - M-E on April 10, 2007 1:01:55 PM]
"Anyone who has never made a mistake has never tried anything new." - Albert Einstein
Advertisement
anyone?
"Anyone who has never made a mistake has never tried anything new." - Albert Einstein
Instead of returning 0..1 object of class PhoneEntry you want to return 0..* entries (where * stands for any number > 0). This could be done as an array but would be elegant since the number of matching results isn't a-priori known. For cases like that Java provides collections. E.g. consider to use java.util.List for this purpose, like so
List<PhoneEntry> search( String targetName1, String targetName2 )

if you use generics or else
List search( String targetName1, String targetName2 )

if not (but I recommend generics). Now, in cases where you have found a match, you have not to return immediately but to append it to the list (which is initially empty after creation) and return the list first after all matches are found. Printing the result then obviously needs an iteration over the list (see java.util.List.iterator() and java.util.Iterator).

Your code as is only works if it is guaranteed that unspecified names are the empty string (and not null or otherwise NullPoinerException will occur). Then you have 3 cases: (1) Both the firstname and the lastname are not the empty string and you test for both. (2) Firstname is the empty string but lastname is not, then you test for the latter one only. (3) Firstname is not empty but lastname is, then you test for the former one only.

So each of the 3 case distinctions is complete and hence the "else" clauses are superfluous. They are not superfluous if you use reduced conditions, e.g. this way:
List<PhoneEntry> result = new List<PhoneEntry>();targetName1 = targetName1.toUpperCase();targetName2 = targetName2.toUpperCase();if(targetName1.length()>0 && targetName2.length()>0) {  // iterate phonebook and test for both firstname and lastname, append on match} else if(targetName1.length()>0) {  // iterate phonebook and test for firstname only, append on match} else if(targetName2.length()>0) {  // iterate phonebook and test for lastname only, append on match}return result;


[Edited by - haegarr on April 10, 2007 1:24:18 PM]
I'm sorry if my answer sounds too abstract but I didn't delved too deeply into your code.

From what I've seen, you have a simple array of PhoneEntry and you perform a linear search through it. Follow the same ideia, but instead of returnig a single entry from the array, return another array with all matches.

PhoneEntry[] searchLastName( String targetName1 ){   retBook = new PhoneEntry[ MAX_OCCURENCES ] ;   int i = 0;    // use linear search to find the targetName    for (int j = 0; j < phoneBook.length; j++) {      if(phoneBook[j].getLastName().toUpperCase().equals(targetName1.toUpperCase())         {              retBook = phoneBook[j];         i++;      }   return retBook;}


You can do the same for empty first names, just swap the member functions on the if clause.

[edit] Oh yeah, there are some bugs in this answer. You can, for example, overrun the array if you have more occurrences in the PhoneBook than the size defined by the MAX_OCCURENCES. So, please, review your options. Using a List is way better, but from the sound of it, you are just learning to deal with arrays, so I kept things as simple as possible.[/edit]
-=-=-=-=-=-=-=-=-=-=-=-=-=--=-=-=-Senior Programmer - iMAX Games
Quote:Original post by HeavyStorm
Oh yeah, there are some bugs in this answer. You can, for example, overrun the array if you have more occurrences in the PhoneBook than the size defined by the MAX_OCCURENCES.

But one can use
PhoneEntry retBook[] = new PhoneEntry[ phoneBook.size ] ;

to overcome the problem. This is of course a horrible memory waste if the phone book becomes large. But as you (HeavyStorm) said, it may be sufficient for learning purposes but IMHO if and only if the OP is aware of the problems arising with such a solution.
Quote:Original post by haegarr
Quote:Original post by HeavyStorm
Oh yeah, there are some bugs in this answer. You can, for example, overrun the array if you have more occurrences in the PhoneBook than the size defined by the MAX_OCCURENCES.

But one can use
PhoneEntry retBook[] = new PhoneEntry[ phoneBook.size ] ;

to overcome the problem. This is of course a horrible memory waste if the phone book becomes large. But as you (HeavyStorm) said, it may be sufficient for learning purposes but IMHO if and only if the OP is aware of the problems arising with such a solution.


Or you could rewrite the function to just display the names as they are found, instead of returning them. Or better yet, generalize that by passing in a functor to invoke on each passed-in name.

BTW, the illustrated code structure is *awful*. You should burn it for teaching you to do this:

public String getFirstName() {  return firstName;}  public String getLastName() {  return lastName;}  public String getPhone() {  return phone;}for (int j = 0; j < phoneBook.length; j++) {// Here I cut out redundant stuff: if the first and last names match, then// it is trivially true that the first name only will match.  if (phoneBook[j].getFirstName().toUpperCase().equals(targetName1.toUpperCase()))         return phoneBook[j];}if ( entry != null )  System.out.println( entry.getFirstName() + " " + entry.getLastName() + ": " + entry.getPhone() );else  System.out.println("Name not found");


You should be doing this sort of thing instead:

public boolean isNamed(String firstName_) {  return firstName.toUpperCase().equals(firstName_.toUpperCase());}  public String toString() {  return firstName + " " + lastName + ": " + phone;}  for (int j = 0; j < phoneBook.length; j++) {  if (phoneBook[j].isNamed(targetName1)) {    return phoneBook[j];  }}System.out.println(entry == null ? "Name not found" : entry.toString());// Or, if you don't mind the output being "(null)" when the name is not found,// you can do it this way:// System.out.println(String.valueOf(entry));


This uses established language idioms (such as the toString() member function - this will also let you use just 'entry' as an argument to println(), BTW, when it's not in a ?: conditional like that), and performs *real* encapsulation of the data members, by delegating the responsibility in a way that actually makes sense and lets you provide meaningful names for the tasks performed by the program. Also, it shortens the complex expressions considerably, which is normally an indication of doing *something* right.

This topic is closed to new replies.

Advertisement