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've tried implementing priority queue with fixed size in Java using TreeSet and overriding add() method:

public class FixedSizePriorityQueue<E> extends TreeSet<E> {
    private final int capacity;

    public FixedSizePriorityQueue(final int capacity) {
        this.capacity = capacity;
    }

    public FixedSizePriorityQueue(
            final int capacity,
            final Comparator<? super E> comparator) {

        super(comparator);
        this.capacity = capacity;
    }

    @Override
    public boolean add(final E e) {
        // initialized with 0 or less than zero capacity
        if (capacity <= 0) {
            return false;
        }

        // keep adding until we fill the queue
        if (size() < capacity) {
            return super.add(e);
        }

        if (comparator() != null
                && comparator().compare(this.last(), e) < 0) {
            pollLast();
            return super.add(e);
        }

        return false;
    }
}

I would really appreciate your thoughts, hints and comments.

share|improve this question
1  
Are you sure you want your FixedSizePriorityQueue be a TreeSet? Do you want to support all methods of a TreeSet ? I would internally use a TreeSet but not extend it. –  MrSmith42 Aug 30 '13 at 10:10
2  
@MrSmith42, good point! I don't need to extend it, I can use it internally. Tnx. –  robosoul Aug 30 '13 at 10:11
    
What's the usage of pollLast() method in add method? –  tintinmj Aug 30 '13 at 14:34
    
@tintinmj the idea is to have fixed size priority queue, and with pollLast(), I remove the most irrelevant element and add new one, using super.add(), leaving super to prioritize. –  robosoul Sep 1 '13 at 13:48
add comment

2 Answers

Two thoughts:

  1. If there is other code and you need all the features of a TreeSet great, otherwise delegate to a TreeSet member variable.

  2. Your code if (capacity <= 0) is somewhat superfluous because of the next test if (size() < capacity)

share|improve this answer
    
thanks for noticing if (capacity <= 0). size() will always return 0 or greater than 0 :) –  robosoul Aug 30 '13 at 10:25
add comment

I rarely check the return of add methods. I would suggest throwing an exception to indicate the container is full. Add methods that return available space and full state.

share|improve this answer
    
Thx for your tip. –  robosoul Sep 11 '13 at 14:21
3  
I didn't downvote on this answer, but this is not what I would consider a "proper" review. Look at some other answers on this site, get a feel for what they should look like, and then come back with that knowledge and integrate it into your answer. –  syb0rg Feb 19 at 0:56
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.