[jadawgis732]

Hi, I'm having a problem with a rather complicated one problem assignment. I'm sure it's an easy fix. Here's an image that should explain the problem. The two linked lists are linked object lists type Person and type Friend.
So we have a friends package consisting of three files (Friend.java Person.java PersonList.java) and a driver.
I'm pretty sure the problem lies in one of the first two methods of PersonList. Here is what I have for that one

package friends;

public class PersonList {
	 Person firstPerson;
	private Person Person;

	public PersonList( ) {
		firstPerson = Person;

	}
	
	public Person lookup(String name){
		Person ptr;
		for (ptr = firstPerson; ptr.nextPerson == null; ptr = ptr.nextPerson) {
			if (ptr.name == null) {
				ptr.name = name;
				return ptr;
			}
			else if (ptr.name == name) {
				return ptr;
			}
		}
		ptr.nextPerson.name = name;
		return ptr;
	}
	
	void addFriend(Person who, String friendName){
		if (firstPerson == null)
			return;
		Person ptr = firstPerson;
		while (ptr != who) {
			ptr = ptr.nextPerson;
		}
		if (ptr == who)
		{
			if (ptr.firstFriend.who == null)
				ptr.firstFriend.who.name = friendName;
			Friend ptr2 = ptr.firstFriend;
			while (ptr2.nextFriend != null) {
				ptr2 = ptr2.nextFriend;
			}
			ptr2.nextFriend.who.name = friendName;
		}
	}
	
		
}
 

Person.java:

package friends;

public class Person {
	String name;
	Friend firstFriend;
	Person nextPerson;
	
	public Person(String name, Person nextPerson) {
		this.name = name;
		this.nextPerson = nextPerson;
		this.firstFriend = null;
	}
	
	public String friendString(){
		String friends = "Friends: ";
		while (this.firstFriend != null) {
			friends += this.firstFriend;
		}		
		return friends;
	}
	
	public void addFriend(Person friend){
// fill in here
	}
	
	public void removeFriend(Person friend){
// fill in here
	}
}

Friend.java

package friends;

public class Friend {
	Friend nextFriend;
	Person who;
	
    Friend(Person who, Friend nextFriend) {
    	this.who = who;
    	this.nextFriend = nextFriend;
   		
	}
	
	void addFriend(Person newWho){
// fill in here
		
	}
		
	Friend findBefore(Person who){
		return null;
		
		
	}
	
	void removeFriend(Person who){
// fill in here
	
	}
}

Here is the driver which I'm pretty sure we cannot modify:

package driver;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import friends.PersonList;
import friends.Person;

public class DriveFriends {
	public static void main(String [] args)
		throws IOException {
		
		BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
		char cmd=' ';
		PersonList people = new PersonList();

		
		while (cmd != 'q'){
			System.out.println("type:  for:");
			System.out.println(" q     quit");
			System.out.println(" a     add friend");
			System.out.println(" d     delete friend");
			System.out.println(" p     print friend list");
			
			Person who;
			String line = br.readLine();
			if (line.length() == 0){
				cmd = ' ';
			} else {
				cmd = line.charAt(0);
			}
			
			switch (cmd){
			case 'a': who = readPerson("person", people, br);
					  who.addFriend(readPerson("friend", people, br));
					  break;
			case 'd': who =  readPerson("person", people, br);
			  	      who.removeFriend(readPerson("friend", people, br));
			  	      break;
			case 'p': who = readPerson("person", people, br);
			 		  System.out.println(who.friendString());
			 		  break;
			case 'q': System.out.println("Bye.");
					  break;
			default:  System.out.println("Bad option.");
			
			}
		}
	}
		
		static Person readPerson(String kind, PersonList people, BufferedReader br)
			throws IOException {
			System.out.println("type "+ kind + "'s name: ");
			Person who = people.lookup(br.readLine());
			return who;
		}
}

Any ideas?

[g00se]

You don't actually say what your problem is, but the basic design is incorrect. A Person and a Friend are essentially the same entity. The difference resides outside the entity itself. You could make this explicit by using a distinct Friendship class as in

public class Friendship<Person, Person> {
..
}

but that could be overkill. Explicit or not, that is the relationship. So a Person should normally have something like

private List<Person> friends; or private Set<Person> friends;

[bcranger]

Well for a start...here's a problem:

You're assignment makes firstPerson null again when the method is finished. Remember objects pass references to their locations so you shouldn't mix up object references or they will catch you.

Here, after the method finishes, firstPerson becomes null. Don't know if you inteded it that way...

Person ptr = firstPerson; 

Also, not sure if you meant it that way but...

Your assigning an object that hasn;t been initialized...

Person firstPerson;
private Person person;

firstPerson = Person;

Mind telling us what the actual problem your having is? Bad output, etc.

[macosxnerd101]

I'm in complete agreement with g00se here. Really, you should have a Group class with List<Person>, and the Person class with a List<Person> representing Friends. As Person and Friend are the same abstractions, I don't see the need for a separate class. Though if you are required to use a Friend class, then Person could have a List<Friend>. But by using a List, you have easier access to the individual Persons and Friends, rather than having to hunt around as you would in a Graph or Tree, which was what your design resembled. Albeit if you use a java.util.LinkedList, then it won't take linear time to access, but it will be a cleaner design.

This illustrates another reason for OOP- encapsulation and modularity. By using a class for your List (even a class from the JDK), you can focus on the abstraction for your Person class without having to worry about the abstraction for the List class.

This post has been edited by macosxnerd101: 15 June 2010 - 03:00 PM

[jadawgis732]

the way the project was assigned was the driver- exactly as you see it, and the three other files. Those all had the fields you see defined at the top. All methods on those 3 files were empty. Some rules:


you must replace each line that says “// fill in here” with appropriate code (The methods in the three files in package friends started like that)
you may add methods as long as they are private. (What does this explicitly prevent me from doing i.e. how does it become harder as a result of this aside from it being compartmentalized a bit more. Is the point just to ensure each method stands on its own?)
Each method must work as specified above. It is not enough that your code works as a whole, because the methods may be tested independently.

So my first problem (I believe) lies in the head of the persons list, called firstPerson. That's just like the head of a linked list correct? It refers to the first Person object in the linked list of Persons.
My second problem is when the driver calls for add friend (there are only 4 choices and one of them is quit, adding a FRIEND (I think) is the only way to input data from the driver). This calls the second method in the PersonList file called Lookup I believe. I must have done that part sloppily because I'm getting null pointers. Can anyone verify that all my constructors are correctly written? If so I guess the lookup method needs some work.

I'm not sure I understand why I'm going about it incorrectly though. If I've been given primitive tools to work with, so be it, I can't change the design.

This post has been edited by jadawgis732: 15 June 2010 - 04:29 PM

[jadawgis732]

bcranger, on 15 June 2010 - 01:30 PM, said:

Your assigning an object that hasn;t been initialized...

Person firstPerson;
private Person person;

firstPerson = Person;

Mind telling us what the actual problem your having is? Bad output, etc.

Null pointer in Line 14/15 of Person List.
I couldn't use Person ptr = new Person as a Person object was defined in the Person File (Thats a class right? I haven't looked at Java since late April.) Eclipse was complaining any time I tried to use "new" referring to an object defined in a separate class. So if I wanted to create a ptr using new would it look like:

Person ptr = new Person.Person

??

This post has been edited by jadawgis732: 15 June 2010 - 04:37 PM

[macosxnerd101]

Quote

(What does this explicitly prevent me from doing i.e. how does it become harder as a result of this aside from it being compartmentalized a bit more. Is the point just to ensure each method stands on its own?)

A private method or variable is only accessible within the class it is defined in. Subclasses cannot even access these methods.

In your List class, this is very bad practice to have a variable named the same thing as its type, as you do with private Person Person;. In addition, you shouldn't be having a Null Pointer (firstPerson) point to another Null Pointer (Person). This is probably causing a lot of Exceptions for you.

         Person firstPerson; 
        private Person Person; 
 
        public PersonList( ) { 
                firstPerson = Person; 
 
        } 

In your addFriend() method, if firstPerson is null, as you leave it when the PersonList is instantiated, addFriend() won't ever function correctly for adding in the first element, meaning you won't be able to add more elems.

for (ptr = firstPerson; ptr.nextPerson == null; ptr = ptr.nextPerson) { 

In your lookup() method, this for loop will almost certaintly give you a NullPointerException, as the condition for continuing is that ptr.nextPerson == null. So when you try to access ptr = ptr.nextPerson, you'll be assigning ptr a null reference. After that, you'll be referencing a variable from a null pointer.

if (firstPerson == null) 
                        return;

If you're stuck with primitive tools or an ill-designed framework, that's not your fault if that's what the teacher is forcing. However, you may want to read up on my Linked List Tutorial, which covers how to implement a Linked List that follows some of the methods outlined in the List interface, keeping to the java.util standard.

[YasuoDancez]

jadawgis732, on 15 June 2010 - 03:35 PM, said:

Person ptr = new Person.Person

??

It should be

Person ptr = new Person( Any Arguments that the person class constructor takes goes here );

In your PersonsList you could do this for the constructor...

public class PersonList {
         Person firstPerson;
        private Person person;

        public PersonList( ) {
           
           addFirstPerson( ); //Initializes the person list with a first person

        }

 //Adds a first person to the list
 private void addFirstPerson()
 {
   person = new Person();
   firstPerson = person;
 }

Don't forget to change the variable Person to a lower case person.

Another style you can do is to do the initializing in the constructor with out a method call to an addFirstPerson() method...

public class PersonList {
         Person firstPerson;
        private Person person;

        //Constructs a new personList
        public PersonList( ) {
           
          person = new Person(); //Init person
           firstPerson = person; //init firstPerson with the new person

        }

@ macosxnerd101
Are you saying it is better to not use the same variable name as the type (

 private Person person 

) or to not use the same variable name that is capitalized? (

 private Person Person 

)I know the capitalization is not good.

This post has been edited by YasuoDancez: 15 June 2010 - 09:25 PM

[macosxnerd101]

YasuoDancez, on 16 June 2010 - 12:23 AM, said:

@ macosxnerd101
Are you saying it is better to not use the same variable name as the type (

 private Person person 

) or to not use the same variable name that is capitalized? (

 private Person Person 

)I know the capitalization is not good.

I'm saying both. As Person is the type, it's bad practice to say Person Person for a variable. As for Java naming conventions and lower-casing variable names, it makes it less confusing for anyone maintaining the code. Remember that we are writing the code not just for the end result program, but for those maintaining our code as well. A philosophy I live by when coding is that the person maintaining my code is a psycho who has my address. Don't really want to piss that person off.

[YasuoDancez]

^ If you put it that way, my fingers may start trembling in nervousness every time i code

Quote

Remember that we are writing the code not just for the end result program, but for those maintaining our code as well.

I hear that saying all the time. It is hard coded into my brain just as Java is becoming.

[jadawgis732]

YasuoDancez, on 15 June 2010 - 08:23 PM, said:

jadawgis732, on 15 June 2010 - 03:35 PM, said:

Person ptr = new Person.Person

??

It should be

Person ptr = new Person( Any Arguments that the person class constructor takes goes here );

In your PersonsList you could do this for the constructor...

public class PersonList {
         Person firstPerson;
        private Person person;

        public PersonList( ) {
           
           addFirstPerson( ); //Initializes the person list with a first person

        }

 //Adds a first person to the list
 private void addFirstPerson()
 {
   person = new Person();
   firstPerson = person;
 }

Thanks fella, you helped me out a bunch there. I was actually able to get away without even using the private field Person person, or whatever we wanted to call the Person object. It now looks like this:

package friends;

public class PersonList {
	 Person firstPerson;
	
	 public PersonList( ) {
	     addFirstPerson( );

	}
	
	public Person lookup(String name){
		Person ptr = new Person("", null);
		for (ptr = firstPerson; ptr.nextPerson != null; ptr = ptr.nextPerson) {
			if (ptr.name == null) {
				ptr.name = name;
				return ptr;
			}
			else if (ptr.name == name) {
				return ptr;
			}
		}
		ptr.name = name;
		return ptr;
	}
		

	void addFriend(Person who, String friendName){
		if (firstPerson == null)
			return;
		Person ptr = firstPerson;
		while (ptr != who) {
			ptr = ptr.nextPerson;
		}
		if (ptr == who)
		{
			if (ptr.firstFriend.who == null)
				ptr.firstFriend.who.name = friendName;
			Friend ptr2 = ptr.firstFriend;
			while (ptr2.nextFriend != null) {
				ptr2 = ptr2.nextFriend;
			}
			ptr2.nextFriend.who.name = friendName;
		}
	}
	 private void addFirstPerson()
	 {
	   Person first = new Person("", null);
	   firstPerson = first;
	 }

	
		
}

, and it is working, compiling and running. Now to just add in the fillers. I appreciate the help once again.
Edit: One quick question, how do I refer to that firstPerson (i.e. the linked list head) from a separate class? It would have to be static in PersonList and that was the one line the instructor wrote in that method...

This post has been edited by jadawgis732: 16 June 2010 - 01:35 AM

[YasuoDancez]

Would you need to access the head from a different class?
Some ways of passing a variable would be to construct a new class inside PersonList and in the new class constructor ( or a method call from that new class ) pass in the head variable as an argument..

Maybe if your list was a doubly linked list you could traverse backwards from a nodes previous node to reach the head.

There is a lecture on youtube from Berkley College called
CS61B Lecture 7: Linked List check it out.

Also you maybe able to shorten your addFirstPerson method to just

 //Adds a first person to the list
 private void addFirstPerson()
 {

   firstPerson = new Person("", null);
 }

and why not do this for your lookup method

public Person lookup(String name){
     Person ptr = new Person("", null);
     for (ptr = firstPerson; ptr.nextPerson != null; ptr = str.nextPerson) {
          if (ptr.name.equals(name) )
              return ptr; //Return ptr if the names are the same           
     }
     return null;//If made it here then there is no next person and then you can do a check outside this method to see if it is null

}

This post has been edited by YasuoDancez: 16 June 2010 - 05:18 AM

[jadawgis732]

Hey OSX - thanks for the link that's going to come in handy. For now, since I'm so deep into this project, I prefer a hands on approach of playing around, seeing what works what doesnt, etc.
Anyway I'm finishing up when I noticed this in the directions:
#

public void addFriend(Person friend): Adds Person friend as a friend of person “this”.
#

public void removeFriend(Person friend): Removes Person friend as a friend of person “this”.

How does this affect me when I'm writing addFriend like so:

public void addFriend(Person friend){
		if (this.firstFriend == null) {
			this.firstFriend.who = friend;
		}
		Friend ptr = this.firstFriend;
		while (ptr.nextFriend.who != null) {
			ptr = ptr.nextFriend;
		}
		ptr.nextFriend.who = friend;
			
		
		
		
		
	}

This post has been edited by jadawgis732: 16 June 2010 - 06:44 AM

[jadawgis732]

YasuoDancez, on 16 June 2010 - 04:01 AM, said:

and why not do this for your lookup method

public Person lookup(String name){
     Person ptr = new Person("", null);
     for (ptr = firstPerson; ptr.nextPerson != null; ptr = str.nextPerson) {
          if (ptr.name.equals(name) )
              return ptr; //Return ptr if the names are the same           
     }
     return null;//If made it here then there is no next person and then you can do a check outside this method to see if it is null

}

Once it's all up on the workspace, and the code works I clean up, right now I'm just working towards a proof-of-concept. After that, OCD kicks in and I nitpick at everything, seriously if I can save one line by reworking 6 I usually do. Meh, everyone has their own workflow... But I will probably end up using similar code

[macosxnerd101]

In response to your NullPointerException in the duplicate topic that was just closed:
The nextFriend variable is an unitialized, null pointer. So when you tell ptr = nextFriend;, you are telling a pointer to reference a null pointer. Then when you try to reference nextFriend from a Null pointer, that is causing your Exception. Also, you are creating a potentially infinite loop below, as you assign ptr = nextFriend, the instance variable only. And since ptr doesn't change what it is referencing, neither will the result of the evaluation of the loop condition.

public class Friend { 
        Friend nextFriend; 
        Person who; 
         
    Friend(Person who, Friend nextFriend) { 
                Friend ptr = who.firstFriend; 
                while (ptr.nextFriend != null) { 
                        ptr = nextFriend; 
                } 

I was in the middle of the reply when it was closed, and I'd rather not see this repsonse go to waste.