Use lists instead of positional arguments to supply optional parameters to methods

[simjnd]

Describe the feature

Instead of using positional arguments for both required and optional parameters, I suggest that we use positional arguments for required parameters and a list at the end for optional parameters, e.g. :

-- from this
Noble.Text.draw(string, x, y, alignment, localized, font)
-- to this
Noble.Text.draw(string, x, y, options)

Of course since this kind of method signature is much more opaque, good documentation is mandatory to be able to know which options keys are available, but IMO this project's great docs make this a non-issue.

Also, eventhough the signature is more ambiguous, reading function calls is actually easier : animation:setState(self.animation.walk, true, self.animation.turn) vs.
animation:setState(self.animation.walk, self.animation.turn, { continuous = true })

---

What problems would this solve or help prevent, if any

This would allow us to supply a value to an optional parameter without having to give one to all of the optional arguments before that we don't care about.

Let's say I'm fine with the default left alignment and unlocalized text, if I want to change the font I currently still have to do this:

Noble.Text.draw("my text", x, y, Noble.Text.ALIGN_LEFT, false, Noble.Text.FONT_LARGE)

Whereas with the proposed change I could do this:

Noble.Text.draw("my text", x, y, { font = Noble.Text.FONT_LARGE })

Additionally this enables the following

• A set of options could be saved to a local variable and then reused easily (e.g. Noble.Text.draw("text", x, y, myConfig))
• This enables us to add more parameters to functions (e.g. set the text color directly in the draw function) without having to worry about a function having way too many parameters
• It can wrap lines a little more gracefully in the case of many arguments

Noble.Text.draw(
  "This is my text which is a little bit longer than usual and makes this a very long line",
  0,
  0,
  Noble.Text.ALIGN_CENTER, true, Noble.Text.FONT_LARGE
)

Noble.Text.draw("This is my text which is a little bit longer than usual and makes this a very long line", 0, 0, {
  alignment = Noble.Text.ALIGN_CENTER,
  localized = true,
  font = Noble.Text.FONT_LARGE
}

---

Additional context

This is also a convention that has been widely adopted in the JavaScript and TypeScript ecosystems

[simjnd]

If you would like an idea of the impact it would have on the codebase, I quickly wrote up what the Text.draw function would look like

Original version

function Noble.Text.draw(__string, __x, __y, __alignment, __localized, __font)
  local alignment = __alignment or Noble.Text.ALIGN_LEFT
  local localized = __localized or false

  if (__font ~= nil) then Graphics.setFont(__font) end -- Temporary font

  if (localized) then
    Graphics.drawLocalizedTextAligned(__string, __x, __y, alignment)
  else
    Graphics.drawTextAligned(__string, __x, __y, alignment)
  end

  if (__font ~= nil) then Graphics.setFont(currentFont) end	-- Reset
end

List version

function Noble.Text.draw(__string, __x, __y, __options)
  local options = __options or {}
  local alignment = options.alignment or Noble.Text.ALIGN_LEFT
  local localized = options.localized or false

  if (options.font ~= nil) then Graphics.setFont(options.font) end -- Temporary font

  if (localized) then
    Graphics.drawLocalizedTextAligned(__string, __x, __y, alignment)
  else
    Graphics.drawTextAligned(__string, __x, __y, alignment)
  end

  if (options.font ~= nil) then Graphics.setFont(currentFont) end	-- Reset
end

[Mark-LaCroix]

I like this, and I can see the benefits, but it feels a bit like an advanced feature that might make the API harder for beginners to learn (although I agree there are benefits to user code legibility). I'd love to hear others weigh in on it.

[jan-martinek]

I’m coming from frontend dev, so I’m not a beginner in this regard, but I‘d welcome the change. I think that while it’s possibly a more complex structure to learn, it’s much easier to debug when learning and trying out things at first — long strings of various booleans and numbers can be very confusing when I’m trying to deduce what I did wrong (especially in languages such as Lua where I’m very lucky if the types do not match and I get a reasonably comprehensible error message).

[ghost]

I think this it's a great feature, I do prefer using named parameters in languages that allows that.

[Mark-LaCroix]

A bit of a non-update on this:

I recently added something like this to the new Noble.setConfig method, but I'm still mulling over the pros/cons of bringing it to other parts of the API. I'm not there yet, but please do continue to try and convince me.

[Mark-LaCroix]

In the time since this was first brought up a bunch of updates have been made and I've never felt a need to expand this to existing systems like Noble.Text, but I still think it has a place in other (or future) parts of the engine where the user should not be expected to provide a value (even nil) for every argument in a method, or when they don't need to even understand every argument.

It's just that IMO there aren't a lot of places where that's true. So it's a no from me on using this for things like Noble.Text.draw(), but I'm still open to this as a pattern elsewhere.

And of course, we're at the point now where enough people are working on or have shipped games with Noble Engine that I'm much less comfortable introducing breaking changes (as a lover of refactoring this breaks my heart).

So, I'm going to close this as an issue, but I'm converting it to a discussion so folks can continue chatting about it, bringing up examples, etc..

[jan-martinek]

Thanks for this. Since my last comment (it’s almost two years? how 🤔), I changed my opinion on this. Currently, I think that because of the CPU cost of table creation and management on Playdate hardware, this technique may be a wrong way to do it.

It could be interesting as an alternative method of calling a specific… method (in cases where it’s easy to keep a single table for multiple calls) — and this could be a way of adding this into the wider codebase. On the other hand, it would definitely be an overhead for code maintenance, so if there isn’t a strong demand, it might be better to drop the idea altogether on principle.