Editing Module talk:TableTools (section)

== Array length merger ==

{{ping|Pppery}} Is there any reason the merged version in the sandbox was never synced with the main template? --[[User:Trialpears|Trialpears]] ([[User talk:Trialpears|talk]]) 20:48, 31 July 2019 (UTC)
: Lack of confidence in my own ability to edit a module used on 8% of all pages without breaking something (Yes, I know I did edit this module in February). The code should be ready to go live. [[User:Pppery|* Pppery *]] [[User talk:Pppery|<sub style="color:#800000">it has begun...</sub>]] 21:02, 31 July 2019 (UTC)
::That is probably the correct response when dealing with these things... Having a second pair of eyes look at it (which won't be me with no module coding experience yet) sounds sensible. Also sorry for deleting your note tat you had merged it, my bad. --[[User:Trialpears|Trialpears]] ([[User talk:Trialpears|talk]]) 22:06, 31 July 2019 (UTC)
::: {{ping|Mr. Stradivarius}} Does the merged code in [[Module:TableTools/sandbox]] ([[Special:Diff/899722883/899724164]]) look good to you? [[User:Pppery|* Pppery *]] [[User talk:Pppery|<sub style="color:#800000">it has begun...</sub>]] 01:43, 23 September 2019 (UTC)
:::: {{ping|Pppery}} Sorry for the late reply. The implementation is mostly good, but for empty arrays it should return 0, whereas currently it returns nil. Also, any chance you could add some test cases for it to [[Module:TableTools/testcases]]? As for naming, I would go with something different. "Binary length" sounds like it could mean the length in bytes of a table (which I know doesn't really have a meaning for Lua tables, but hey). It also isn't clear that this only works for arrays or array-like tables. Finally and most nit-pickingly, it isn't ''technically'' a [[binary search]]; [[exponential search]]es and binary searches are slightly different. How about "arrayLength" instead? The explanation of which algorithm is used would have to be relegated to the doc page, but I think this might be a clearer approach overall. Best — '''''[[User:Mr. Stradivarius|<span style="color: #194D00; font-family: Palatino, Times, serif">Mr.&nbsp;Stradivarius</span>]]''''' <sup>[[User talk:Mr. Stradivarius|♪&nbsp;talk&nbsp;♪]]</sup> 08:53, 29 September 2019 (UTC)
::::: {{ping|Mr. Stradivarius}} OK, I've [[Special:Diff/918679523|fixed the 0 versus nil bug]], and [[Special:Diff/587868130/918677165|added some test cases]]. As to the naming, I don't like <code>arrayLength</code>, because it is not meaningfully distinguishable from the pre-existing <code>length</code> function, and two functions having names that are synonyms feels like bad coding style. [[User:Pppery|* Pppery *]] [[User talk:Pppery|<sub style="color:#800000">it has begun...</sub>]] 20:57, 29 September 2019 (UTC)
:::::: {{ping|Pppery}} Ah, I had forgotten that there was also a <code>length</code> function. I agree that it is confusing to have both <code>length</code> and <code>arrayLength</code>. How about replacing <code>length</code> with <code>binaryLength</code>? It is a much more efficient algorithm for large arrays, and only marginally less efficient for very small arrays; plus it has the same requirement that no intermediate elements in the array can be nil. If any code is affected by switching from an incremental to an exponential algorithm, that code was probably broken to begin with. — '''''[[User:Mr. Stradivarius|<span style="color: #194D00; font-family: Palatino, Times, serif">Mr.&nbsp;Stradivarius</span>]]''''' <sup>[[User talk:Mr. Stradivarius|♪&nbsp;talk&nbsp;♪]]</sup> 14:25, 30 September 2019 (UTC)
:::::: Also, thank you for the algorithm fix and for adding the test case. I split the test case up into four separate test cases; this should make it easier to pinpoint the error if a future edit causes a problem with the function. — '''''[[User:Mr. Stradivarius|<span style="color: #194D00; font-family: Palatino, Times, serif">Mr.&nbsp;Stradivarius</span>]]''''' <sup>[[User talk:Mr. Stradivarius|♪&nbsp;talk&nbsp;♪]]</sup> 14:27, 30 September 2019 (UTC)
::::::: {{ping|Mr. Stradivarius}} I didn't go with that originally because [[Module:Array length]] has text on its documentation page saying it shouldn't be used for small arrays but yes, that suggestion does seem reasonable. [[User:Pppery|* Pppery *]] [[User talk:Pppery|<sub style="color:#800000">it has begun...</sub>]] 21:13, 30 September 2019 (UTC)
::::::: Replacement of <code>length</code> with <code>binaryLength</code> done. [[User:Pppery|* Pppery *]] [[User talk:Pppery|<sub style="color:#800000">it has begun...</sub>]] 18:46, 5 October 2019 (UTC)
{{od}} And {{done|merged}} with main module. Let's see if anything breaks ... [[User:Pppery|* Pppery *]] [[User talk:Pppery|<sub style="color:#800000">it has begun...</sub>]] 03:41, 21 December 2019 (UTC)