Shuffle
Shuffle copied to clipboard
Vestride
Sorting: number of calls to compare() increased for v6.0.0 compared to v5.4.0: 790 vs 110 times!
Shuffle version v5.4.0 and v6
Describe the bug
When asking Shuffle v6.0.0 to sort the tiles, e.g. with respect to 'groups' (as per Shuffle's examples page), I see my own-defined compare() being called a lot more times than needed or at least more than when using older Shuffle v5.4.0:
v6.0.0 calls my own-defined compare(a,b) 790 times for 12 tiles.
v5.4.0 calls it only 110 times (for same 12 tiles).
when tiles are 4 I get 15 times vs 10 times.
Additionally and equally important, I also see differences in the number of calls between Firefox v88.0 and Chrome v94.0 (chrome fares better, above numbers are for Firefox).
20min Edit: Perhaps the sort algorithm is stochastic and there is some difference in numbers between runs, but the numbers above are consistent for small number of tiles. For larger number of tiles numbers do vary but v6.0.0 has the highest always (I did a few runs but I can not call my testing exhaustive!). Thanks.
Reproduction link
To Reproduce
Reproduce the example in https://vestride.github.io/Shuffle/ under "Advanced sorting" and implement the following basic compare() function with a basic counter - don't forget to reload the page and reset the counter for each new experiment! :
var shufflecount = 0;
shuffleInstance.sort({
compare: (a, b) => {
shufflecount++;
console.log("called for "+a.element.dataset.groups+" vs "+b.element.dataset.groups+", count is "+shufflecount);
if( a.element.dataset.groups > b.element.dataset.groups ){ return 1; }
if( a.element.dataset.groups < b.element.dataset.groups ){ return -1; }
return 0;
} // end compare
});
...
Expected behavior
I am not sure how many comparisons (how many times the compare: (a, b) => {} should be called) but v5.4.0 calls it much less than v6.0.0 and with correct results, based on my examples.
Desktop (please complete the following information):
- Browser: Firefox v88.0 and Chrome v94.0
- OS: Linux latest
Thanks for your response. Can you please clone https://codepen.io/Vestride/pen/ZVWmMX , but make it use Shuffle v6.0.0? Then I can add a compare() and test both (I assume that fiddle uses Shuffle v5 - which one though?) Alternatively, I have been looking for a really condensed, minimal, self-contained HTML+JS snippet which I can use as a skeleton for the testing in my browser. But I could not find any, hence the lack of test cases. My code is difficult to condense and isolate the ShuffleJS part. So send me a skeleton or make a fiddle for me to use as a testbed.
I updated both the homepage demo in codepen to v6 as well as the codepen template.
Here's the old v5 homepage pen and old v5 template
Thank you for this. I can not reproduce what I claimed above when running a test using the fiddles you provided. Both v5 and v6 produce same results, for 12 tiles there are 42 pair-wise comparisons which seems reasonable.
So it's probably a false alarm from my side. I am sorry.
I will rewrite how I instantiate Shuffle in my code for v6.0.0 following the fiddle you provided.
I will update this issue in a couple of days.
I have managed to reduce my code into a very condensed self-contained html below. The problem still happens with the following code.
I may very well suffer from cargo-culting or mindless copy-paste. If you are kind enough to check if this is how I should be using Shuffle and its sort() capability?
Many thanks.
<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml" lang="en">
<head>
<meta charset="utf-8">
<script language="javascript" src="https://cdnjs.cloudflare.com/ajax/libs/Shuffle/6.0.0/shuffle.min.js"></script>
</head>
<body>
<div class="shuffle-container" id="my_shuffle_container">
<div class="col-1@sm col-1@xs my-sizer-element"></div>
<!-- <div class="shuffle-item" id="item12" data-code="item12">item12</div><br/>
<div class="shuffle-item" id="item11" data-code="item11">item11</div><br/>
<div class="shuffle-item" id="item10" data-code="item10">item10</div><br/>
<div class="shuffle-item" id="item9" data-code="item9">item9</div><br/>
<div class="shuffle-item" id="item8" data-code="item8">item8</div><br/>
<div class="shuffle-item" id="item7" data-code="item7">item7</div><br/>
<div class="shuffle-item" id="item6" data-code="item6">item6</div><br/>
<div class="shuffle-item" id="item5" data-code="item5">item5</div><br/>
<div class="shuffle-item" id="item4" data-code="item4">item4</div><br/>
-->
<div class="shuffle-item" id="item3" data-code="item3">item3</div><br/>
<div class="shuffle-item" id="item2" data-code="item2">item2</div><br/>
<div class="shuffle-item" id="item1" data-code="item1">item1</div><br/>
</div>
<script>
// On each reload the Demo class is created and a sort() is done
document.addEventListener("DOMContentLoaded", function(){
if( document.getElementById('my_shuffle_container') === null ){ throw "failed to find element with id 'my_shuffle_container' which contains the shuffle items, you should not then try to instantiate Shuffle."; }
class Demo {
constructor(){
// counts how many times compare() was called, it's reset on window reload
this.counter = 0;
this.element = document.querySelector('.shuffle-container');
if( this.element === null ){ throw ".shuffle-container not found"; }
this.shuffle = new Shuffle(this.element, {
itemSelector: '.shuffle-item',
sizer: this.element.querySelector('.my-sizer-element')
});
if( this.shuffle === null ){ throw "new Shuffle() has failed"; }
this.sortoptions = {
randomize: false,
by: null,
compare: (a,b) => {
this.counter++;
console.log("compare called for "+a.element.dataset.code+" and "+b.element.dataset.code+", count: "+this.counter);
if( a.element.dataset.code > b.element.dataset.code ){ return 1;}
if( a.element.dataset.code < b.element.dataset.code ){ return -1;}
return 0;
}
};
} // end constructor()
dosort(){ this.shuffle.sort(this.sortoptions); }
}; // end class Demo
// instantiate the Demo class above and do a sort(), only on reloading the window
window.myDemo = new Demo();
window.myDemo.dosort();
});
</script>
</body>
</html>
For 3 shuffle items, the result is this:
compare called for item3 and item2, count: 1
compare called for item3 and item1, count: 2
compare called for item2 and item1, count: 3
compare called for item3 and item2, count: 4
compare called for item3 and item1, count: 5
compare called for item2 and item1, count: 6
Can you try using the initialSort option?
Instantiating Shuffle will call the sort method already, so calling dosort after that would be doubling everything.
Looks like there is no change: sort() and sorter() are called twice with below code which uses initialSort and does not call sort() directly.
I have placed a throw new Error(); in sorter() and with below code it shows this:
sort() called. shuffle.v6.0.0.js:1539:10
Uncaught Error:
sorter /shuffle.v6.0.0.js:398
sort /shuffle.v6.0.0.js:1548
filter /shuffle.v6.0.0.js:1530
_init /shuffle.v6.0.0.js:794
Shuffle /shuffle.v6.0.0.js:758
Demo /with_initialsort.html:48
<anonymous> /with_initialsort.html:58
shuffle.v6.0.0.js:398:7
sort() called. shuffle.v6.0.0.js:1539:10
Uncaught Error:
sorter /shuffle.v6.0.0.js:398
sort /shuffle.v6.0.0.js:1548
update /shuffle.v6.0.0.js:1583
layout /shuffle.v6.0.0.js:1594
onLoad /shuffle.v6.0.0.js:779
So, even without a direct sort() call it sorts twice, once on _init() and once on onLoad().
<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml" lang="en">
<head>
<meta charset="utf-8">
<script language="javascript" src="https://cdnjs.cloudflare.com/ajax/libs/Shuffle/6.0.0/shuffle.min.js"></script>
</head>
<body>
<div class="shuffle-container" id="my_shuffle_container">
<div class="col-1@sm col-1@xs my-sizer-element"></div>
<!--
<div class="shuffle-item" id="item12" data-code="item12">item12</div><br/>
<div class="shuffle-item" id="item11" data-code="item11">item11</div><br/>
<div class="shuffle-item" id="item10" data-code="item10">item10</div><br/>
<div class="shuffle-item" id="item9" data-code="item9">item9</div><br/>
<div class="shuffle-item" id="item8" data-code="item8">item8</div><br/>
<div class="shuffle-item" id="item7" data-code="item7">item7</div><br/>
<div class="shuffle-item" id="item6" data-code="item6">item6</div><br/>
<div class="shuffle-item" id="item5" data-code="item5">item5</div><br/>
<div class="shuffle-item" id="item4" data-code="item4">item4</div><br/>
-->
<div class="shuffle-item" id="item3" data-code="item3">item3</div><br/>
<div class="shuffle-item" id="item2" data-code="item2">item2</div><br/>
<div class="shuffle-item" id="item1" data-code="item1">item1</div><br/>
</div>
<script>
// On each reload the Demo class is created and a sort() is done
document.addEventListener("DOMContentLoaded", function(){
if( document.getElementById('my_shuffle_container') === null ){ throw "failed to find element with id 'my_shuffle_container' which contains the shuffle items, you should not then try to instantiate Shuffle."; }
class Demo {
constructor(){
// counts how many times compare() was called, it's reset on window reload
this.counter = 0;
this.element = document.querySelector('.shuffle-container');
if( this.element === null ){ throw ".shuffle-container not found"; }
this.sortoptions = {
randomize: false,
by: null,
// by: function(element){ console.log(" by called for "+element.dataset.code); return element.dataset.code; }
compare: (a,b) => {
this.counter++;
console.log("compare called for "+a.element.dataset.code+" and "+b.element.dataset.code+", count: "+this.counter);
if( a.element.dataset.code > b.element.dataset.code ){ return 1;}
if( a.element.dataset.code < b.element.dataset.code ){ return -1;}
return 0;
}
};
this.shuffle = new Shuffle(this.element, {
itemSelector: '.shuffle-item',
sizer: this.element.querySelector('.my-sizer-element'),
initialSort: this.sortoptions
});
if( this.shuffle === null ){ throw "new Shuffle() has failed"; }
} // end constructor()
dosort(){ this.shuffle.sort(this.sortoptions); }
}; // end class Demo
// instantiate the Demo class above and do a sort(), only on reloading the window
window.myDemo = new Demo();
});
</script>
</body>
</html>
Thank you for your time, this is not an urgent problem for me. I will have a look at the Shuffle source and try to find what's happening...
Just to be on the same page, I am using https://cdnjs.cloudflare.com/ajax/libs/Shuffle/6.0.0/shuffle.min.js - which one does your fiddle use? Because I also saw a discrepancy in the number of compare() calls (which was my initial problem). The fiddle calls it 42 times for 12 tiles, below code calls it 50 times (twice).