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

Add util.In( any thing, vararg values ) #2056

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bydlocodd
Copy link

@bydlocodd bydlocodd commented Feb 21, 2024

Allows you to check if thing is among values.
Using the function, we can do this:

local myUsergroup = 'user'
if util.In( myUsergroup, 'user', 'vip' ) then
    -- ...
end

... instead of:

local allowedUsergroups = {
    user = true,
    vip = true,
}

local myUsergroup = 'user'
if allowedUsergroups[ myUsergroup ] then
    -- ...
end

... or:

local myUsergroup = 'user'
if myUsergroup == 'user' or myUsergroup == 'vip' then
    -- ...
end

Of course, the second code is faster because it performs a key lookup (which is O(1) vs. my O(n) implementation), but util.In is useful in contexts where performance is not so important

Allows you to check if `thing` is among `values`.
Using the function, we can do this:

```lua
local myUsergroup = 'user'
if util.In( curState, 'user', 'vip' ) then
    -- ...
end
```

... instead of:
```lua
local allowedUsergroups = {
    user = true,
    vip = true,
}

local myUsergroup = 'user'
if allowedUsergroups[ myUsergroup ] then
    -- ...
end
```

Of course, the second code is faster because it performs a key lookup (which is O(1) vs. my O(n) implementation), but `util.In` is useful in contexts where performance is not so important
@robotboy655
Copy link
Collaborator

Bad naming aside,

local myUsergroup = 'user'
if table.HasValue( { 'user', 'vip' }, myUsergroup ) then
    -- ...
end

I understand this is less efficient than the proposed method, but by your own admission that is not the focus here.

@Kefta
Copy link
Contributor

Kefta commented Feb 21, 2024

Not creating the table would indeed be much more efficient when JIT compiles the select calls. Not as efficient as creating a hashtable with true vals, but for dynamic vararg values, this seems like the best solution. Efficiency should be the focus of this addition.

@@ -408,3 +408,15 @@ function util.IsBinaryModuleInstalled( name )

return false
end

function util.In( thing, ... )
for i = 1, select( '#', ... ) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think it was Spar who demonstrated that localizing your for limit makes it marginally faster.

Something like:

Suggested change
for i = 1, select( '#', ... ) do
local limit = select( "#", ... )
for i = 1, limit do

might be a nice microoptimization

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how this is an improvement since the loop limit is only evaluated once.
image

Copy link

@Grocel Grocel Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how this is an improvement since the loop limit is only evaluated once.

Does it behave like that in GLua too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@bydlocodd
Copy link
Author

bydlocodd commented Feb 21, 2024

Not sure how I can improve on my solution ( except for brandonsturgeon's suggestion). Of course, pretty much the same thing can be done using table.HasValue (honestly, I just forgot this function existed), but my method is a bit faster.

Regarding the function name - I was guided by Python syntax:

if thing in (a, b, c):
    # ...

@bydlocodd
Copy link
Author

bydlocodd commented Feb 21, 2024

Bad naming aside,

local myUsergroup = 'user'
if table.HasValue( { 'user', 'vip' }, myUsergroup ) then
    -- ...
end

I understand this is less efficient than the proposed method, but by your own admission that is not the focus here.

Yes, performance is not a focus, because in that case I will use a key lookup. However, the developed solution, although similar to table.HasValue, is slightly more optimized. In this case, I see my function as a compromise between readability and speed - we sacrifice speed for readability, but not as much as in the case of using table.HasValue

@robotboy655 robotboy655 added the Addition The pull request adds new functionality. label Feb 27, 2024
@TankNut
Copy link

TankNut commented Mar 10, 2024

With regards to the name, util.InValues?

@Heyter
Copy link

Heyter commented Mar 14, 2024

util.In:
        sum = 0.286
        avg = 0.00286
        median = 0.0030000000000001
util.In2:
        sum = 0.0020000000000024
        avg = 2.0000000000024e-05
        median = 0
function util.In2( thing, ... )
  local iMax, i = select( '#', ... ), 1

  ::iter::
  if thing == select( i, ... ) then
    return true
  end
  i = i + 1
  if i <= iMax then goto iter end

    return false
end

@Grocel
Copy link

Grocel commented Mar 14, 2024

util.In:
        sum = 0.286
        avg = 0.00286
        median = 0.0030000000000001
util.In2:
        sum = 0.0020000000000024
        avg = 2.0000000000024e-05
        median = 0
function util.In2( thing, ... )
  local iMax, i = select( '#', ... ), 1

  ::iter::
  if thing == select( i, ... ) then
    return true
  end
  i = i + 1
  if i <= iMax then goto iter end

    return false
end

Sample size and size of ...?

@Heyter
Copy link

Heyter commented Mar 14, 2024

Sample size and size of ...?

lib: https://github.com/Be1zebub/Small-GLua-Things/blob/master/sh_benchmark.lua

util.In:
        sum = 0.035
        avg = 0.00035
        median = 0
util.In2:
        sum = 0.00099999999999767
        avg = 9.9999999999767e-06
        median = 0
        
	benchmark("util.In", function()
		a = util.In(userName, "admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user",		userName)
	end, 1000, 100)
	benchmark("util.In2", function()
		a = util.In2(userName, "admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "admin", "test", "user", "admin", "test", "user", "admin", "test", "user","admin", "test", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user", "user",  userName)
	end, 1000, 100)

table unpack

util.In:
        sum = 0.113
        avg = 0.00113
        median = 0.0010000000000048
util.In2:
        sum = 6.415
        avg = 0.06415
        median = 0.063000000000017
        
local a = false
local users = {}
local userName = "superadmin"

for i = 1, 253 do
	if i % 2 == 0 then
		users[i] = "admin"
	else
		users[i] = "user"
	end
end

users[254] = 'superadmin'

benchmark("util.In", function()
	a = util.In(userName, unpack(users))
end, 1000, 100)
benchmark("util.In2", function()
	a = util.In2(userName, unpack(users))
end, 1000, 100)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Addition The pull request adds new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants