Sign in to follow this  
M-E

Java problem

Recommended Posts

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]

Share this post


Link to post
Share on other sites
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]

Share this post


Link to post
Share on other sites
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[i] = 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]

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this