1

For a programming assignment, I have been working on creating a program that reads an input file and sorts the data inside using a self-made max heap priority queue. The data file has lines that either read "insert [a name] [a number]", or "remove". For this priority queue, we need to make a function for inserting and removing objects. Each object in the queue contains the name as a string, and the priority as a integer. I have to implement this heap based on an array with a size of 255.

My question is, I'm having difficulty implementing my insert and remove functions to work as specified. I'll provide 1) how they need to work, 2) pseudocode I've made, and 3) the actual Java code I've implemented. Both of my functions do not work exactly as I intend for them to, so I could use some direction from more experienced programmers.

1) insert(name, priority): this function should take a name of type string and a priority of type integer, and inserts them into the priority queue. remove(): this function should remove the object with the highest priority value and return the name string from the object.

2) As background, I have three classes for this program: First, the "main" class containing implementation for reading the file and using the functions. Second, the "name" class, which creates the name object containing the name string and priority int , a constructor, and a compareTo method for comparing the priority values of two objects. Third, the "priorityqueue" class, contains the insert and remove functions. Now, here is the pseudocode I made for those two functions:

insert:

  • Check if the array is full (when num = 255), throw if true
  • Create the object from the input file with a name string and priority int
  • Insert the object at num
  • Use a while loop to swap the two objects at insertion
  • Update num (num++)

remove:

  • Save the first object
  • Move the last object to the first
  • Update num (num--)
  • Use a while loop to determine the larger child and return it.

3) Here is the code I have so far. I'll provide my name and priority queue classes, in case my name class is what's giving me trouble.

Priority Queue class:

public class PriorityQueue 
{
int num; //amount of things in array 
int idx; //index of current name object
Name[] names = new Name[255];

public void insert(String name, int priority)
{
    while (num != 255)
    {
        Name addName = new Name(name, priority);
        names[num] = addName;
        num++;

        while(idx != 0 || names[idx].CompareTo(names[(idx-1)/2]))
        {
            Name temp = names[idx];
            names[idx] = names[(idx-1)/2];
            names[(idx-1)/2] = temp;

            idx = (idx-1)/2;
        }
    }
}

public Name remove()
{
    Name temp2 = names[0];
    //Save first element

    names[0] = names[idx];
    //Move last element to first

    num--;
    while(idx < Math.max(2*idx+1,2*idx+2))
    {
        if(names[idx].CompareTo(names[(idx-1)/2]))
                {
                    Name temp3 = names[idx];
                    names[idx] = names[(idx-1)/2];
                    names[(idx-1)/2] = temp3;
                }
    }
    return temp2;
}

}

Name class:

public class Name implements Comparable
{
String name;
int priority;

public Name(String n, int i)
{
    name = n;
    priority = i;
}

public boolean CompareTo(Name obj)
{
    if(priority < obj.priority)
    {
        return false;
    }

    else if(priority > obj.priority)
    {
        return true;
    }

    return true;
}
}

I appreciate any help. Thanks!

3
  • Surely this is wrong: while (num != 255) {. And you said "throw if true", no trace of that in the code. Commented Oct 31, 2016 at 19:11
  • @MarkoTopolnik: You are correct that I forgot the throw. But why do you say the while condition is incorrect? One of the specifications for this assignment is our queue contain no more than 255 elements, so I want this loop to run until num is greater than 255, since it represents the number of elements in the queue. Commented Oct 31, 2016 at 20:27
  • Because it's in the method whose job is to insert one entry into the heap. Commented Oct 31, 2016 at 20:30

1 Answer 1

0

Several problems. First, in your insert method:

public void insert(String name, int priority)
{
    while (num != 255)
    {
        Name addName = new Name(name, priority);
        names[num] = addName;
        num++;

        while(idx != 0 || names[idx].CompareTo(names[(idx-1)/2]))
        {
            Name temp = names[idx];
            names[idx] = names[(idx-1)/2];
            names[(idx-1)/2] = temp;

            idx = (idx-1)/2;
        }
    }
}

The while (num != 255) shouldn't be there. You should check to see if num == 255, and throw an exception if it is.

Then, you need to initialize idx. That is:

        Name addName = new Name(name, priority);
        names[num] = addName;
        idx = num;
        num++;

And your while condition should use && rather than ||. Otherwise you'll do the swap every time idx is not equal to 0.

In your remove method:

public Name remove()
{
    Name temp2 = names[0];
    //Save first element

    names[0] = names[idx];
    //Move last element to first

    num--;
    while(idx < Math.max(2*idx+1,2*idx+2))
    {
        if(names[idx].CompareTo(names[(idx-1)/2]) > 0)
                {
                    Name temp3 = names[idx];
                    names[idx] = names[(idx-1)/2];
                    names[(idx-1)/2] = temp3;
                }
    }
    return temp2;
}

You don't want names[idx] there, because you don't know what idx is. You want:

names[0] = names[num-1]; // get last item in the heap

Your while condition here is goofy. Math.max(2*idx+1,2*idx+2) will always return 2*idx+2, unless idx is negative. And, again, you haven't even initialized idx. What you want here is:

idx = 0;
while (idx < num)

Now, what you're trying to do is see if names[idx] is smaller than either of its children. And, if so, select the largest of the two children to swap it with. So:

while (idx < num)
{
    int largestChild = idx*2+1;
    if (largestChild >= num) break; // idx is at a leaf level
    if (largestChild+1 < num)
    {
        // compare the two children
        if (names[largestChild].compareTo(names[largestChild+1]) < 0)
        {
            largestChild = largestChild+1;
        }
    }
    if (names[idx] < names[largestChild])
    {
        // swap, moving the item down
        temp = names[idx];
        names[idx] = names[largestChild];
        names[largestChild] = temp;
        idx = largestChild;
    }
    else
    {
        // item is in the proper place
        break;
    }
}

I would suggest making idx a method-scoped variable in both methods. There's no need for it to be a global, and making it local to the methods forces you to initialize it before you use it, rather than potentially (as in your existing code) using a stale value.

I think you need to change your Name class's CompareTo function. The Comparable compareTo function must return:

a negative integer, zero, or a positive integer as this object is less than, equal to, or greater than the specified object.

So you should have:

public boolean CompareTo(Name obj)
{
    if(priority < obj.priority)
    {
        return -1;
    }

    if (priority > obj.priority)
    {
        return 1;
    }

    return 0;
}
Sign up to request clarification or add additional context in comments.

13 Comments

Thanks so much for your help, @Jim Mischel! I have one more question for you involving my PriorityQueue class. I assume I need at least one constructor in order to print the heap in my main class. If so, what input arguments should the constructor take? Would it be the Name object, or the string name and int priority (which obviously the Name object contains, but I've attempted both ways and keep getting a null pointer error)?
@vertoslash: By the way, I think your implementation of Comparable is wrong. Shouldn't it be int compareTo(Name obj) as described here?
@vertoslash: I don't see a constructor anywhere. Are you asking about your PriorityQueue constructor? I'd think something like PriorityQueue myQueue = new PriorityQueue(); would be sufficient. Then, do all operations on the myQueue reference.
One question about your while loop for the remove function. In netbeans, I get an error on the if statement: if(names[largestChild] < names[largestChild+1]) { largestChild = largestChild+1; } Netbeans says I can't compare these with a < operand, since the arrays both contain Name objects instead of integers. So how should I do the comparison here?
@vertoslash: Probably something like if (names[largestChild].CompareTo(names[largestChild+1]) < 0). You'll also need to change the comparison in the insert. See my note about changing your compareTo method.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.