Don't sort a column with same values
yairEO opened this issue · 19 comments
for a column whom all its values are the same, do not bother sorting. Touching the DOM for no reason costs in performance and if better to avoid.
Here's a quick idea how to check if all columns values are the same:
var table = document.querySelector('table'),
rowsCount = table.rows.length,
values = {};
// lets say a user is sorting the first column
// skip the first tr which only has "th"..
while( rowsCount-- > 1){
values[ table.rows[rowsCount].children[0].textContent ] = 1;
}
// same value if the object has less than 2 keys
if( Object.keys(values).length < 2 )
return; // do not sort
Hi there!
In my opinion, the probability that a row has the same value for each column is fairly low. Doubling the number of comparisons for each sort on each column to optimize for a statistically unlikely case seems inefficient.
What do you think?
I'm being a system for a client now which has many tables...and I opened this issue because statistically I do get data quite often where a column's cells are completely the same.
I've tried to come up with a fix which is the least harmful to performance. not using jQuery at all, doing a reverse loop, and using an object to check if there are multiple values. I would say, it would be a good idea to at least have this as an optional thing, that can be switched "on" via settings.
I will think of a way to accommodate this without making the plugin do too much. I want to keep the plugin as basic as possible. There are currently a lot of PRs outstanding with small features being added. I believe the reason why there are so many PRs is exactly because the source is small enough to alter on an as-needed basis.
I would suggest then something else:
Make a core file which contains the most basic, then instead of merging all those great PRs and ideas, just add them as "plugin" files, so a person would be able to download a certain file and extend the core code.
For example, the core would be:
stupidtable.js
And my suggestion above would be:
stupidtable.columns-with-same-value.js
As long as you change the core to make it flexible enough to add functionality from outside which can work much like how jQuery's plugins work.
I realize this is a lot of work, but since this repo is pretty popular (for a good reason) I would say this will give the repo a great edge of others which does similar things.
Perhaps making the stupidsort() method composed of more overwrite-able methods would accommodate this without too much clutter being added. Then the "plugins", such as your example, could just be a series of method overrides.
Great! I think it's also important to have them in the repo so developers won't waste time writing them again and again, and could just copy what they need to extend your code, like a library of optional methods.
👍 Let's make it happen. Closing this issue for now - we will reopen with a more general plugin architecture issue and reference all currently opened issues that feature tiny improvements.
I thought this was a duplicate of a currently opened issue, but there are closed issues (such as #141) that relate. I'll go ahead and keep this one open to represent the "don't sort columns with equal values" feature
What exactly are you trying to achieve with this? I don't see how looping through an entire table to check if the values are the same is any more efficient than just sorting to start with.
A better solution would be to simply not add the "data-sort" attribute in the first place when the values are identical.
If the table is generated server-side this is trivial. If you're generating the table in JS it's still fairly straightforward, you may just need to remove the attribute instead. Seems more sensible to me to check each column once rather than on every attempted sort.
@svivian -
looping is very fast and designed code design is optimal.
A better solution would be to simply not add the "data-sort" attribute in the first place when the values are identical.
How would you know if you should insert data-sort or not if you won't loop and check all values are the same or not? you could loop on your own data, yes, but I think a universal, out-of-the-box solution which fits everybody is better.
in Theory, per-developer, it's better to remove the data-sort yourself and check the data, as you suggested, but that solution would be duplicated all over the world again and again by devs instead of it being automated once in this plugin.
How would you know if you should insert data-sort or not if you won't loop and check all values are the same or not?
I never said not to loop. I'm saying to do this check when the table is created, not in JS when sorting. If the table is created server-side then you can loop there (where it will be faster, and you only do it once anyway).
Another concern is that custom sorting functions may wish to handle equal values in some other way. For example, if two values are equal maybe they check another column to decide which order to put them.
To go back to the original question: what exactly are you trying to achieve? What is the benefit of not sorting when the values are the same? You save a few milliseconds? There is also a UX argument against doing nothing when the user expects something to happen.
@svivian -
For columns which has the same value, but have a mix of values in other columns, I do not wish for them to be sorted, and they currently get. rows move around for no apparent reason. The logic of the sorting seems unknown and one will ask themselves what's happening here. (UX-wise)
As I said before, I don't want to manually check this when I render my tables, because it's better to have a solution out-of-the-box for the countless people who might use the plugin and wish this functionality. you cannot expect thousands of people to go and change their template files and logic to solve a problem that would be instantly solved with updating this script.. seems a better way of doing things for the general public and saving time for everybody, while also keeping your own code's logic as simple as possible and not adding more to it.
@svivian shares my vision of wanting to keep this plugin as small and simple as possible. The core of the plugin should be stupid. I've kept the plugin so stupid, in fact, that I've even rejected the idea of a settings object. But throughout the years, we've seen different needs pop up that the stupid table doesn't quite accommodate.
While the simple code within the stupid table has allowed for many to make the revisions they've needed, there are likely many other devs who do not have the skill or confidence to alter the source themselves when the stupid table doesn't quite fit their needs. In my opinion, our plugin has excluded this group for long enough. I believe a simple method-overriding mechanism with a sufficient number of hooks can allow for the creation of solutions to specific problems while keeping the core beautifully stupid.
Incorporating these plugins could be as simple as
<script src="stupidtable.js"></script>
<script src="plugins/prevent-equal-sorting.js"></script>
<script src="plugins/blank-boxes-on-top.js"></script>This is the direction I'd like the project to go. I believe it keeps the spirit of the project intact while allowing for developers to easily customize the experience for their users. In the end, our project is all about how easily the stupid table plugin can allow developers to create the experience they have in mind for their users.
Hey @yairEO. No progress on this on my end. I have a kid on the way, I probably won't have time to get to this for a while. I'd be glad to review a PR that implements this though.
I'll look into that.
Hopefully it will be done before you kid will start sorting tables of his/her own and need this feature ;)
I will be using tables for changing rather than sorting in the near future 💩
I ended up writing this plugin :
$.fn.stupidtable_plugins['prevent-equal-sorting'] = function(){
var table = this,
rowsCount = table.rows.length,
sortBtns = $(table.tHead).find('.sortBtn').removeClass('sortBtn');
// check each sortable column
sortBtns.closest('th').each(function(i, th){
var diversityObj = {},
colIdx = $(th).index(),
rows = rowsCount;
// check each row, per column
while( rows-- > 1 ){
diversityObj[ table.rows[rows].children[colIdx].textContent ] = 1;
// check for column diversity. if found, stop checking current column
if( Object.keys(diversityObj).length > 1 ){
sortBtns.eq( colIdx ).addClass('sortBtn');
break;
}
}
})
}
And I thought to call plugins in this manner:
$.fn.stupidtable_plugins = {};
$.fn.stupidtable = function(sortFns) {
return this.each(function() {
...
for( var plugin in $.fn.stupidtable_plugins ){
$table.stupidtable_plugins[plugin].call(this);
}
});
};
Currently this works in my env.
I think this solves some issues of adding functionality, but does not solve issues like when there's a need to change the runSort function itself. I thought maybe it would be better to expose the runSort as a jquery $.fn method (in some manner), so it could be easily overridden with a custom one.
what do you think?
It took me a while but I cooked up a plugin/settings architecture and had this request implemented. See the should_redraw setting.
Thanks to everyone for the great discussion on this issue.