I need some help!

[Brusalk]

Hey There!

So I've written this function to be used in one of my addons. It accepts a bunch of tables, and then returns a table containing all values common to every table passed in.

Now, where I'm confused is that it's seemingly never reaching a point in execution, even though it should be.

local function tableMerge(...) 
	if select('#', ...) < 3 then error("tableMerge", "inputs") return end
	local toReturn = {}
	for i=1, select('#', ...) do
		local tab = select(i, ...)
		if type(tab) == "table" then --ignore any inputs which aren't tables
			if not toReturn[1] then
				for j,spellbar in pairs(tab) do -- do it this way so we don't actually do anything to the tables passed in
					table.insert(toReturn, spellbar)
					print("Adding " .. spellbar)
				end
			else
				for j,v in pairs(toReturn) do -- for everything that's common up to this point in iteration:
					print("Testing " .. v)
					local found = false
					for k,spellbar in pairs(tab) do -- for everything in this new table to check
						if v == spellbar then		-- if we found that the spellbar exists in this new table, say we found it
							found = true
							print("Found " .. spellbar)
						end
					end
					if not found then -- if we didn't find this element, then remove it from the common table
						print("Removing " .. toReturn[j])
						table.remove(toReturn, j)
					end
				end				
			end			
		end
	end
	return toReturn -- return everything that's common
end

function testMerge()
	return tableMerge({1,2,3,4,5,6,7}, "test", {2,4,6}, {})
end

Here's example output of what testMerge dumps:
Dump: value=testMerge()
Adding 1
Adding 2
Adding 3
Adding 4
Adding 5
Adding 6
Adding 7
Testing 1
Removing 1
Testing 3
Removing 3
Testing 5
Removing 5
Testing 7
Removing 7
Testing 2
Removing 2
Testing 6
Removing 6
[1]={
[1] = 4
}


I'm very perplexed because it's never actually finding any common elements, even though it should be (It's not removing 2,4,6 when it reaches the 3rd table)

What's even more perplexing to me is that it's never removing the 4th value, even though it should be as the last table is empty.

I've been trying to debug this for over an hour now and I can't figure out why it's behaving like it is. (setting the last table to 2 causes the output to be {2,6}



Any help you guys can give me I would be VERY appreciative!

-Brusalk

[kaels]

Your iterator is getting mucked up because you're using pairs over a table whose indexes are changing inside the loop. As a rule, if you're iterating over a table with pairs or ipairs, you should try to avoid using table.remove and table.insert on that table inside the loop to avoid the sort of unpredictable behaviour you're seeing here.

Try this - it's definitely not optimized, but it should work:

local function tableMerge(...) 
    if select('#', ...) < 3 then error("tableMerge", "inputs") return end
    local toReturn = {}
    for i=1, select('#', ...) do
        local tab = select(i, ...)
        if type(tab) == "table" then --ignore any inputs which aren't tables
            if not toReturn[1] then
                for j,spellbar in pairs(tab) do -- do it this way so we don't actually do anything to the tables passed in
                    table.insert(toReturn, spellbar)
                    print("Adding " .. spellbar)
                end
            else
                local n = #toReturn -- get the size of the array
                -- iterate over the indices in the array
                for j = 1,n do -- for everything that's common up to this point in iteration:
                    local v = toReturn[j] -- get the value at this index
                    print("Testing " .. v)
                    local found = false
                    for k,spellbar in pairs(tab) do -- for everything in this new table to check
                        if v == spellbar then       -- if we found that the spellbar exists in this new table, say we found it
                            found = true
                            print("Found " .. spellbar)
                        end
                    end
                    if not found then -- if we didn't find this element, then remove it from the common table
                        print("Removing " .. toReturn[j])
                        toReturn[j] = nil -- set to nil to remove without changing the array
                    end
                end
                -- now we need to clean up the table
                for j = 1,n do -- iterate over the array again
                    local v = toReturn[j] -- get the value at this index
                    if v then -- look for non-empty values
                        toReturn[j] = nil -- clear the current index
                        table.insert(toReturn, v) -- reinsert the value at the first non-empty index
                    end
                end             
            end         
        end
    end
    return toReturn -- return everything that's common
end

And here's an alternative that may be quicker (only runs a cleanup function once) but won't preserve order, and might actually be slower (at least for a small number of tables) because it has to use pairs:

local function tableMerge(...) 
    if select('#', ...) < 3 then error("tableMerge", "inputs") return end
    local toReturn = {}
    local n -- define n outside the loop so we can use it later
    for i=1, select('#', ...) do
        local tab = select(i, ...)
        if type(tab) == "table" then --ignore any inputs which aren't tables
            if not toReturn[1] then
                for j,spellbar in pairs(tab) do -- do it this way so we don't actually do anything to the tables passed in
                    table.insert(toReturn, spellbar)
                    print("Adding " .. spellbar)
                end
                n = #toReturn -- get the size of the initial table - it won't get any bigger than this
            else
                -- we have to iterate with pairs because after the first run, our table will have empty indices
                for j, v in pairs(toReturn) do -- for everything that's common up to this point in iteration:
                    print("Testing " .. v)
                    local found = false
                    for k,spellbar in pairs(tab) do -- for everything in this new table to check
                        if v == spellbar then       -- if we found that the spellbar exists in this new table, say we found it
                            found = true
                            print("Found " .. spellbar)
                        end
                    end
                    if not found then -- if we didn't find this element, then remove it from the common table
                        print("Removing " .. toReturn[j])
                        toReturn[j] = nil -- set to nil to remove without changing the array
                    end
                end
                -- note that the resulting table has a bunch of empty indices after each loop           
            end         
        end
    end
    -- now we can do a single cleanup run
    for j = 1, n do -- iterate over the maximum size of the table
        local v = toReturn[j] -- get the value at the current index
        if v then -- if we have a non-nil value 
            toReturn[j] = nil -- remove the value from its current index
            table.insert(toReturn, v) -- put the value in the first empty index
        end
    end
    return toReturn -- return everything that's common
end

You'll want to check for typos, I haven't tested either code.

[Brusalk]

Thanks a bunch!

I ended up going with the second one, but I had to make one change to make sure it works right:

local function tableMerge(...) 
	if select('#', ...) < 3 then error("tableMerge", "inputs") return end
	local toReturn = {}
	for j,spellbar in pairs(...) do -- do it this way so we don't actually do anything to the tables passed in
		table.insert(toReturn, spellbar)
		print("Adding " .. spellbar)
	end
	local n = #toReturn -- get the size of the initial table - it won't get any bigger than this
	for i=2, select('#', ...) do
		local tab = select(i, ...)
		if type(tab) == "table" then --ignore any inputs which aren't tables
				-- we have to iterate with pairs because after the first run, our table will have empty indices
			for j, v in pairs(toReturn) do -- for everything that's common up to this point in iteration:
				print("Testing " .. v)
				local found = false
				for k,spellbar in pairs(tab) do -- for everything in this new table to check
					if v == spellbar then		-- if we found that the spellbar exists in this new table, say we found it
						found = true
						print("Found " .. spellbar)
					end
				end
				if not found then -- if we didn't find this element, then remove it from the common table
					print("Removing " .. toReturn[j])
					toReturn[j] = nil -- set to nil to remove without changing the array
				end
			end
			-- note that the resulting table has a bunch of empty indices after each loop			
		end
	end
	-- now we can do a single cleanup run
	for j = 1, n do -- iterate over the maximum size of the table
		local v = toReturn[j] -- get the value at the current index
		if v then -- if we have a non-nil value	
			toReturn[j] = nil -- remove the value from its current index
       		table.insert(toReturn, v) -- put the value in the first empty index
       	end
	end
	return toReturn -- return everything that's common
end

function testMerge()
	return tableMerge({1,2,3,4,5,6,7}, "test", {2,4,6}, {2,4})
end

I had to move the init of toReturn outside of the loop and just start iterating at the second input. Since the loop was able to remove the first entry it was adding the whole table back in again if the iteration nil'd the first entry in toReturn.

Simple fix though. Thank you!

[Vrul]

Originally Posted by Brusalk I'm very perplexed because it's never actually finding any common elements, even though it should be (It's not removing 2,4,6 when it reaches the 3rd table)

Originally Posted by Brusalk What's even more perplexing to me is that it's never removing the 4th value, even though it should be as the last table is empty.

Those two quotes contradict one another. Since one of your inputs is an empty table you should never have any common values in the output. The reason you do get an output is somewhat answered by kaels, you are using table.remove on a table you are iterating over via pairs(). As table.remove can change table indexes and that causes undefined behavior (i.e. unexpected results) during ipairs/pairs iterations.

The fix is simple though. You are mixing dictionary indexing with array indexing in the same loop for no apparent reason. You know toReturn is an array as you set it up that way so you should always treat it that way. All you need to do is iterate over the table in reverse order so that table.remove (tremove to avoid an unnecessary table look up) doesn't throw everything off when it shifts the contents of the table.

Reusing your code with some minor tweaks:

local function tableMerge(...)
	if select('#', ...) < 3 then error("tableMerge", "inputs") return end
	local toReturn = { }
	for i = 1, select('#', ...) do
		local tab = select(i, ...)
		if type(tab) == 'table' then --ignore any inputs which aren't tables
			if toReturn[1] then
				for index = #toReturn, 1, -1 do -- for everything that's common up to this point in iteration:
					local value, found = toReturn[index], false
					print("Testing " .. value)
					for _, spellbar in pairs(tab) do -- for everything in this new table to check
						if value == spellbar then		-- if we found that the spellbar exists in this new table, say we found it
							found = true
							print("Found " .. spellbar)
							break
						end
					end
					if not found then -- if we didn't find this element, then remove it from the common table
						print("Removing " .. value)
						tremove(toReturn, index)
					end
				end				
				if not toReturn[1] then break end
			else
				for _, spellbar in pairs(tab) do -- do it this way so we don't actually do anything to the tables passed in
					toReturn[#toReturn + 1] = spellbar
					print("Adding " .. spellbar)
				end
			end			
		end
	end
	return toReturn -- return everything that's common
end

I also threw in a fix for a bug in your code:

function testMerge()
	return tableMerge({1,2,3,4,5,6,7}, "test", {}, {2,4,6})
end

That input in your original code (providing it worked as intended) would've had an output of {2,4,6}.

[Brusalk]

Sorry. What I meant was that it wasn't finding any common elements before getting to the end of execution. So, when it was going from the first table of values to the second, it should've found some common ones and printed found: #, but it wasn't.