Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Bug in SlotList#insertWithPriority #56

Closed
thanhbv opened this Issue Sep 13, 2011 · 3 comments

Comments

Projects
None yet
2 participants

thanhbv commented Sep 13, 2011

I think (for sure) in latest source (#c184b20c6f) , there's a bug in SlotList#insertWithPriority:
correct code must be:

if (priority > current.head.priority){
    subClone.tail = current.prepend(slot);
    return wholeClone; 
}

NOT:

if (priority > current.head.priority){
    const newTail:SlotList = current.prepend(slot);
    return new SlotList(head, newTail);
}

(the slots from this.tail to current's previous slot are omitted!)

Owner

robertpenner commented Sep 13, 2011

Thanks for your attention to this. Are you able to write a unit test that confirms this as a bug?

thanhbv commented Sep 15, 2011

[Test]
public function insertWithPriority():void{
    var s:PrioritySignal = new PrioritySignal();
    var l1:Function = function():void{};
    var l2:Function = function():void{};
    var l3:Function = function():void{};
    var l4:Function = function():void{};
    var slot1:ISlot = new Slot(l1, s);
    var slot2:ISlot = new Slot(l2, s, false, -1);
    var slot3:ISlot = new Slot(l3, s);
    var slot4:ISlot = new Slot(l4, s);
    var list:SlotList = new SlotList(slot1);
    list = list.insertWithPriority(slot2);
    list = list.insertWithPriority(slot3);
    list = list.insertWithPriority(slot4);
    assertEquals(4, list.length);
    assertEquals(slot1, list.head);
    assertEquals(slot3, list.tail.head);
    assertEquals(slot4, list.tail.tail.head);
    assertEquals(slot2, list.tail.tail.tail.head);
}
Owner

robertpenner commented Sep 20, 2011

Thanks for the unit test and corrected code. Worked perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment