Collections.sort() not sorting with compareTo Override

As the title states – here’s my Packet class – I’m trying to sort into ascending order of packetNum:

public class Packet implements Comparable<Packet>{
    private short packetNum;
    private short authKey;
    private byte[] audio;

    public Packet()
    {
        packetNum = 0;
        authKey = 0;
        audio = null;
    }

    public Packet(short packetNum, short authKey, byte[] audio)
    {
        this.packetNum = packetNum;
        this.authKey = authKey;
        this.audio = audio;
    }

    @Override
    public int compareTo(Packet other) {
        int cmp = 0;
        if (this.packetNum < other.packetNum) {
            cmp = -1;
        }
        if (this.packetNum == other.packetNum) {
            cmp = 0;
        }
        else {
            cmp = 1;
        }
        return cmp;
    }
}

And here’s my sorting code in another class’ main (inside a while loop):

//Packet constructed
Packet received = new Packet(packetNumReceived, authKeyReceived, encryptedAudio);

//Get packet num
short packetNum = received.getPacketNum();

//Hold block for reordering (16 packets)
ArrayList<Packet> block = new ArrayList<Packet>();

while (running) {
    //Add packet to ArrayList
    block.add(received);
    System.out.println(packetNum);
    //Re-order packets
    if (block.size() == 16) {
        Collections.sort(block);
        for (int i = 0; i < block.size(); i++) {
            //print out the sorted packet numbers
            System.out.println(block.get(i).getPacketNum());
            player.playBlock(block.get(i).getAudio());
        }
        block.clear();
    }
}

The packet numbers printed are in the same (incorrect) order, before and after the sort. I’ve also check the array elements directly, and the order is not changed at all. These sections of code are the only time the Packet class is touched/referenced at all, not sure what I’m doing wrong. These are my only 2 classes, and there are no reused variable names across them.

Answer

@Override
public int compareTo(Packet other) {
    int cmp = 0;
    if (this.packetNum < other.packetNum) {
        cmp = -1;
    }
    if (this.packetNum == other.packetNum) {
        cmp = 0;
    }
    else {
        cmp = 1;
    }
    return cmp;
}

In this code, you are returning 1 if this.packetNum == other.packetNum gives you false, even if you wanted to return -1.

You forgot an else:

    (...) 
    else if (this.packetNum == other.packetNum) {
        cmp = 0;
    }
    (...)

Leave a Reply

Your email address will not be published. Required fields are marked *