Revise entry sorting: refactor for readability, remove duplicates, sort rest alphabetically #52
Conversation
…rt rest alphabetically Duplicates can occur for variables that are also selectors in untyped models, or also due to custom hooks.
|
Please discuss #39 (comment) before approving this. :) |
There was a problem hiding this comment.
Overall, nice refactoring, no objections to removing duplicates in principle. It shouldn't hurt performance though if possible.
If it's not too much trouble, could you benchmark this by creating a fake ECModel instance with random entries and running the old and new sortEntries functions?
If performance does indeed degrade, it may be enough and faster to only remove duplicates that are displayed directly after one another.
e.g. filter out:
aCuefollowed byaCue
As that should be doable in O(n) with low constant overhead and covers the most obvious case as described in #39
May even be doable in the rendering code, which should be really fast 🤔
| entries sort: | ||
| [:entry | entry matchNarrowString: narrowString] descending | ||
| , [:aEntry :bEntry | aEntry <= bEntry ifTrue: [-1] ifFalse: [0]] |
There was a problem hiding this comment.
Where does the , operator on BlockClosure come from???
Is that new? It doesn't work on my system.
Or am I reading the syntax incorrectly?
There was a problem hiding this comment.
Ah, okay, descending returns a PropertySortFunction that can be comma-chained...
That's wild 😅, but okay.
| | seen | | ||
| entries sort: | ||
| [:entry | entry matchNarrowString: narrowString] descending |
There was a problem hiding this comment.
Huh, cool, no idea #descending worked on BlockClosure 👍
|
|
||
| "remove any duplicates" | ||
| seen := Set new. |
There was a problem hiding this comment.
I don't mind removing duplicates, as discussed in #39 , duplicates are just a side-effect of adding all selectors in the untyped model.
There's no real point in displaying them.
How is performance though? First sorting, then converting into a Set sounds a bit expensive 🤔
Especially in the untyped model that contains all selectors.
Duplicates can occur for variables that are also selectors in untyped models, or also due to custom hooks.