Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

table.rotate should use the triple reverse method. #56

Open
Parakleta opened this issue Mar 25, 2022 · 1 comment
Open

table.rotate should use the triple reverse method. #56

Parakleta opened this issue Mar 25, 2022 · 1 comment

Comments

@Parakleta
Copy link

The existing shift/push method touches every element in the table once per step. The triple reverse method touches every element exactly twice.

function tablex.rotate(t, n)
    local tlen = #t
    local mid = -n % tlen
    tablex.reverse(t)
    tablex.reverse(t, 1, mid)
    tablex.reverse(t, mid+1, tlen)
end

For this to work your tablex.reverse needs to take optional bound arguments. Note that tablex.reverse should also be caching the length value rather than recalculating it in the loop.

@1bardesign
Copy link
Owner

should probably branch on n being 1 or not, because that's the most common case in my experience and it'd be strictly slower there.

i think i'd probably prefer tablex.reverse_span rather than reverse itself taking optional arguments, so the intent is clear.

i'm fine with this change otherwise 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants