Module talk:Lua class

Globals

@Alexiscoutinho: Further to our discussion at Module talk:TableTools#Shouldn't isArray be more generic?, I had a quick look at this very nice module. There may be a couple of issues for you to contemplate. First, many modules require Module:No globals. If one of those modules tried to use something from here, it would be likely to crash because there are several globals in this module. Are they really needed, or should it return a table containing tables of functions + classes + instances? Second, are you aware that in function try_parser, calls to type() are calling Lua's function, not the _G.type modified function? Johnuniq (talk) 10:43, 11 July 2021 (UTC)

@Johnuniq: The globals probably aren't required, but are extremely recommended. I would prefer if such modules dropped their use of Module:No globals if they were to use classes. This module is meant to fundamentally change how other modules are structured and coded so it's only fitting for them to switch to globals. I wasn't aware of that type bug. I knew I shouldn't call it below that function override, but I completely forgot that the parser called it during the pre-submit check. I haven't extensively tested this module by the way. I was hoping to initially test it through examples and actual usage in other basic type modules (upcoming). Alexiscoutinho (talk) 05:24, 12 July 2021 (UTC)

The problem is that people have no way of detecting typos which generate unintended globals, so Module:No_globals is fairly common and may be becoming more so. I don't use it because I have a script which shows globals. I don't mind, I just thought I would point out the issue. Re "type", I would rename the variable to lua_type so there cannot be any confusion in the code or in the reader. Johnuniq (talk) 05:31, 12 July 2021 (UTC)

@Johnuniq: Well, I think the ideal solution would be to open more exceptions (besides "arg") in Module:No globals and/or require it after Lua class. Alexiscoutinho (talk) 06:06, 12 July 2021 (UTC)

That's up to you, I just wanted to make sure you were aware of the issue. At any rate, if you ever wanted, you could alter the way it works in the future. Johnuniq (talk) 07:26, 12 July 2021 (UTC)

The convention with Lua is to minimize the use of globals. See [1] and also [2] which is the style guide used by LuaRocks. Gonnym (talk) 14:06, 13 July 2021 (UTC)

Sure, I'll try to minimize globals. Current new globals are:

• class
• issubclass
• isinstance
• try

Planned optional globals basic types are:

• frozenset
• set
• list
• tuple

Alexiscoutinho (talk) 23:13, 21 July 2021 (UTC)

If you want this module to be widely used, there should be no globals. Why are they needed? It's cleaner to use the "namespace" of a required module and write x.class (where x is the table returned by require('Module:Lua class')). Johnuniq (talk) 02:42, 22 July 2021 (UTC)

Globals are a bad thing, please avoid them. As a bare minimum make it possible to override them. To clarify, globals make hard to test code and code that somehow use those globals become non-portable. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 16:52, 18 December 2022 (UTC)

elseif

Several of the elseif can be avoided, as the previous clause exit's from further evaluation. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 16:56, 18 December 2022 (UTC)

assert

There are several occurrences of a test clause followed by an error statement. These can be replaced by assert, thus avoiding the explicit if clause. That is assert(<test>, <message>). The only place where an explicit error might be correct is when a redirect on level is appropriate, that is the second argument to error(<message>, <level>). 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 17:01, 18 December 2022 (UTC)

libraryUtil.checkTypeMultiForIndex

There are several libraryUtil.checkTypeMultiForIndex after argument is first used. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 17:06, 18 December 2022 (UTC)

type

Reimplementing standard functions are a bad thing, it makes the code fragile, non-portable, and outright dangerous to use as it can make unrelated code running in the same context to fail. This has implications for Module:Lua_set in addition for the one on the subject page. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 17:12, 18 December 2022 (UTC)

refactor class, possibly also constructor

The class function is way to complex. The constructor also border on being to complex. 2001:4644:13BE:0:E955:D3DE:360E:9F1D (talk) 18:29, 18 December 2022 (UTC)