NobleEngine icon indicating copy to clipboard operation
NobleEngine copied to clipboard

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

Open simjnd opened this issue 1 year ago • 5 comments

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 avatar Sep 02 '22 15:09 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

simjnd avatar Sep 02 '22 16:09 simjnd

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.

Mark-LaCroix avatar Sep 05 '22 21:09 Mark-LaCroix

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).

jan-martinek avatar Sep 07 '22 11:09 jan-martinek

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

ussaohelcim avatar Oct 05 '22 00:10 ussaohelcim