Editing Module talk:TableTools (section)

== Shouldn't isArray be more generic? ==

{{ping|Pppery|Johnuniq|Izno}} Currently, isArray raises an error if the input is not a table, shouldn't it just return false, which is what a user would expect from an isXxx function? Furthermore, wouldn't it be better if it checked if the input was array-like? This would allow custom objects/containers to be considered array-like if they are iterable and only have numeric keys. [[User:Alexiscoutinho|Alexiscoutinho]] ([[User talk:Alexiscoutinho|talk]]) 14:23, 9 July 2021 (UTC)
:I agree that isArray should return true or false and should never raise an error—that would be much more in keeping with Lua's style of doing the right thing when reasonable. The checkType should be replaced with a line to return false if the input is not a table. BTW, that's an amazing implementation which would never have occurred to me. I see [[#Bug in deepCopy]] above has not been addressed yet. [[User:Johnuniq|Johnuniq]] ([[User talk:Johnuniq|talk]]) 00:13, 10 July 2021 (UTC)
::{{re|Johnuniq}} What do you think of this?
<syntaxhighlight lang="lua">
function (obj)
	if not pcall(pairs, obj) then
		return false
	end
	local i = 0
	for _ in pairs(obj) do
		i = i + 1
		if obj[i] == nil then
			return false
		end
	end
	return true
end
</syntaxhighlight>
:::Good, but I would stick with:
<syntaxhighlight lang="lua">
function p.isArray(t)
	if type(t) ~= 'table' then
		return false
	end
	local i = 0
	for _ in pairs(t) do
		i = i + 1
		if t[i] == nil then
			return false
		end
	end
	return true
end
</syntaxhighlight>
:::That's using <code>t</code> for simplicity and consistency with the rest of the module. It would not be possible for <code>pcall(pairs, obj)</code> to be successful unless <code>obj</code> is a table so whereas it is more pure to test for that, it is clearer to admit that a table is what works. I see that the function returns true if given <code>{}</code> (an empty table) and that seems desirable. [[User:Johnuniq|Johnuniq]] ([[User talk:Johnuniq|talk]]) 07:16, 10 July 2021 (UTC)
::::{{re|Johnuniq}} <code>pcall(pairs, obj)</code> would work if <code>obj</code> implements <code>__pairs</code>. Although <code>obj</code> would technically still be a table, it could have a custom type (by overriding <code>type</code>, which is what [[Module:Lua class]] does). Do you think it would be better to name my function something different (isArrayLike) then? The original one could have your tweak too. [[User:Alexiscoutinho|Alexiscoutinho]] ([[User talk:Alexiscoutinho|talk]]) 15:23, 10 July 2021 (UTC)
:::::Yikes, I hadn't absorbed that [[Module:Lua class]] was seriously implementing classes. The question becomes a matter of what approach would be best in the long term, given that modules are (or will be) maintained by Wikipedians who might struggle when faced with code that has no apparent starting point or path of execution (which is the consequence of fully using classes). Also, consistency is important and isArray would look out-of-place in [[Module:TableTools]] if it is the only function with <code>obj</code> as the parameter and which tests for the method it is going to use rather than the variable type. I'm highly pragmatic and would use the naive code I put above, but you have a reason for wanting the proper implementation and that's ok by me. Regarding pragmatism, the heading comment for isArray (with its mention of a table) might be confusing, but a fully correct comment would be three times longer with the new code. Regarding duplicating the function, that's ugly as I'm sure you know. I haven't done anything with this module and I'm not sure why I'm watching here, although I have commented a couple of times. In other words, I'm happy for you to do whatever is wanted. It might be best to try something in the sandbox, preferably also fixing the deepCopy bug I mentioned, then update the module. [[User:Johnuniq|Johnuniq]] ([[User talk:Johnuniq|talk]]) 02:21, 11 July 2021 (UTC)
::::::I personally think that learning to maintain class based modules is just as easy as function based ones. For more complex modules, I think OOP greatly improves readability/understanding for anyone for many reasons, e.g. variables wouldn't have to be carried everywhere in function signatures. Of course, simpler modules might not need classes. It's just a matter of using it where it's appropriate. I agree it wouldn't be symmetrical if <code>isArray</code> received <code>obj</code>, thus I will propose <code>isArrayLike</code> instead. Since these functions are so small, I don't think a small duplication would be bad. The two functions would have their own different use cases too. [[User:Alexiscoutinho|Alexiscoutinho]] ([[User talk:Alexiscoutinho|talk]]) 21:04, 11 July 2021 (UTC)
:::::::To stick up for functions, at least you can tell what the inputs are! I'm happy for you to edit as wanted and there doesn't appear to be anyone else with an opinion so please go for it. [[User:Johnuniq|Johnuniq]] ([[User talk:Johnuniq|talk]]) 22:32, 11 July 2021 (UTC)
@Alexiscoutinho: I checked your edit to [[Module:TableTools/sandbox]] and it looks good, thanks. You will see that I removed trailing whitespace and removed two unused functions added to the sandbox on 22 February 2021 because I don't think we should add functions without a specified use. After my minor tweaks, along with your comment changes (a very good idea to make them consistent), it is now a nightmare to compare the main module with the sandbox, but I believe the change is good.
{{#invoke:convert/tester|compare|TableTools}}
[[User:Johnuniq|Johnuniq]] ([[User talk:Johnuniq|talk]]) 05:37, 12 July 2021 (UTC)

=== Protected edit request on 25 July 2021 ===
{{edit fully-protected|Module:TableTools|answered=y}}
This request is to implement all the changes made to the sandbox until this time. The differences can be checked with the diff link of the above section. Summarizing changes: improved module formatting; improved isArray; added isArrayLike; fixed _deepCopy; and improved defaultKeySort. [[User:Alexiscoutinho|Alexiscoutinho]] ([[User talk:Alexiscoutinho|talk]]) 14:27, 25 July 2021 (UTC)
:Sorry this has taken so long to get to. {{done}}, on assurances this has been tested by you and also checked by Johnuniq. &mdash;&nbsp;Martin <small>([[User:MSGJ|MSGJ]]&nbsp;·&nbsp;[[User talk:MSGJ|talk]])</small> 10:42, 4 October 2021 (UTC)