Module talk:Color contrast

Making the lum function accessible from modules

I've made the lum function accessible from modules by changing the code from:

function p.lum(frame)
	return color2lum(frame.args[1] or frame:getParent().args[1])
end

to:

function p.lum(frame)
	local args = frame.args[1] or frame:getParent().args[1]
	return p._lum(args)
end

function p._lum(args)
	return color2lum(args)
end

The changes are on the sandbox and will implement them if there are no comments. --Gonnym (talk) 11:03, 6 January 2019 (UTC)[reply]

@Gonnym: That looks like a very sensible and useful improvement. My only reservation is that passing the parameters as a table gives the person wanting to import the routine no idea of what values/types to supply to the routine without having to read through all of the code to determine them. In general I'd usually recommend either documenting a brief list of required values for the args table or passing the parameters as a list of obviously-named variables rather than a table. But that's just a minor point and shouldn't stop you from updating the main module. --RexxS (talk) 13:03, 6 January 2019 (UTC)[reply]

@RexxS: I don't mind doing either if that helps. I just followed the current style used for the other three public functions. Do you want me to change _lum(args) to _lum(color)? --Gonnym (talk) 13:27, 6 January 2019 (UTC)[reply]