Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: Go 2: Array or variadic destructuring to discrete variables #56094

Closed
eaglebush opened this issue Oct 7, 2022 · 9 comments
Closed
Labels
dotdotdot ... FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal Proposal-FinalCommentPeriod v2 An incompatible library change
Milestone

Comments

@eaglebush
Copy link

Author background

  • Would you consider yourself a novice, intermediate, or experienced Go programmer?
    I consider myself as novice to intermediate Go programmer. I have no extensive experience of looking at established Go code under the hood.
  • What other languages do you have experience with?
    I have VB, C# and PHP experience before becoming interested in Go.

Related proposals

  • Has this idea, or one like it, been proposed before?

    I'm not sure. I searched the proposals and I can't find a proposal that discussed the same.

  • Does this affect error handling?. I'm not sure.

  • Is this about generics? No.

Proposal

  • What is the proposed change?

I would like to suggest a language change that can execute dynamic assignments to a list of discrete variables.
I have the following code:

 // Process options
if len(opts) == 1 {
  switch t := opts[0].(type) {
  case string:
	  if size64, err := strconv.ParseInt(t, 10, 32); err == nil {
		  size = int(size64)
	  }
  case int:
	  size = t
  default:
	  size = -1
  }
}

if len(opts) == 2 {
  switch t := opts[1].(type) {
  case string:
	  if size64, err := strconv.ParseInt(t, 10, 32); err == nil {
		  scale = uint8(size64)
	  }
  case int:
	  scale = uint8(t)
  case uint8:
	  scale = t
  default:
	  scale = 0
  }
}

if len(opts) == 3 {
  switch t := opts[2].(type) {
  case string:
	  if size64, err := strconv.ParseInt(t, 10, 32); err == nil {
		  prec = uint8(size64)
	  }
  case int:
	  prec = uint8(t)
  case uint8:
	  prec = t
  default:
	  scale = 0
  }
}

The opts variable is a variadic parameter in a function:

func NewExtParam(value interface{}, opts ...interface{}) ExtParam {
}

In order to check if one or more of the values have been set, I need to check the length of the opts variable in order to enter and process the value further. As you have noticed, there is no null checking yet.

It would be nice if we can destructure an array or a variadic variable like the one below:

sz, sc, pr := opts...

And then we can write the following check if the value is null or not.

if sz != nil {
  // process further
}
if sc!= nil {
  // process further
}
if pr != nil {
  // process further
}
  • Who does this proposal help, and why?

It can help developers to simplify their code

  • Please describe as precisely as possible the change to the language.

  • What would change in the language spec?

  • Please also describe the change informally, as in a class teaching Go.

    The mechanics can be as follows:

    • It will only set the values specified in left-to-right order. Meaning, if there is no third (3) variable in the set, the assignment would not be prompted. A similar feature of this is the assertion operator or the map's second result.
    • A null value would be read if the destination is more than the length of the variadic or array input
  • Is this change backward compatible?

    • Breaking the Go 1 compatibility guarantee is a large cost and requires a large benefit.
      Show example code before and after the change.
    • Before
    • After
  • Orthogonality: how does this change interact or overlap with existing features?

  • Is the goal of this change a performance improvement?

    • If so, what quantifiable improvement should we expect?
    • How would we measure it?

Costs

  • Would this change make Go easier or harder to learn, and why?

I think it will contribute to a more idiomatic Go concept. Instead of manually checking the length and indexing an array like the code I gave above, a quick and easy assignment like this would lessen the clutter.

PHP has the list function. It made the values retrieval easy to code and understand.

  • What is the cost of this proposal? (Every language change has a cost).
  • How many tools (such as vet, gopls, gofmt, goimports, etc.) would be affected?
  • What is the compile time cost?
  • What is the run time cost?
  • Can you describe a possible implementation?
  • Do you have a prototype? (This is not required.)
@gopherbot gopherbot added this to the Proposal milestone Oct 7, 2022
@rittneje
Copy link

rittneje commented Oct 7, 2022

For the particular use case you mentioned, why would you not just use a struct instead of variadic parameters?

type ExtParam struct {
    Size uint8
    Scale uint8
    Prec uint8
}

This has the side benefit of improving clarity on the caller side, as people don't have to figure out which parameter is which anymore.

@DeedleFake
Copy link

This has the side benefit of improving clarity on the caller side, as people don't have to figure out which parameter is which anymore.

It also allows them to be different types and have clearly defined definitions for one being unset.

@eaglebush
Copy link
Author

eaglebush commented Oct 8, 2022

For the particular use case you mentioned, why would you not just use a struct instead of variadic parameters?

type ExtParam struct {
    Size uint8
    Scale uint8
    Prec uint8
}

This has the side benefit of improving clarity on the caller side, as people don't have to figure out which parameter is which anymore.

You're right, it could be coded like that by adding a struct next to the value argument. But then I needed the NewExtParam function to be called by just the value argument. It's a sort of a habit coming from VB and C# languages that allows an optional argument with a default value if omitted.

The idea of the proposal came as I tried to handle the omitted arguments. I just thought it would be great if we have a PHP-styled destructuringlist() function made into the language itself.

@ianlancetaylor ianlancetaylor added LanguageChange Suggested changes to the Go language v2 An incompatible library change labels Oct 8, 2022
@ianlancetaylor
Copy link
Contributor

I feel like your new code looks so much shorter than your old code because you left out a bunch of lines that would have been repeated.

Also your new code implies that the values are being set to a pointer type, as you are comparing them to nil. Are they type *uint8?

@rittneje
Copy link

rittneje commented Oct 8, 2022

But then I needed the NewExtParam function to be called by just the value argument.

I'm not clear what specifically NewExtParam is returning in your original snippet, but another common pattern I've seen is this:

type ExtParam struct {
    value interface{}
    size uint8
    scale uint8
    prec uint8
}

type ExtParamOption func(*ExtParam)

func WithSize(size uint8) ExtParamOption {
    return func(p *ExtParam) { p.size = size }
}

...

func NewExtParam(value interface{}, opts ...ExtParamOption) ExtParam {
    extParam := ExtParam{value: value}
    for _, opt := range opts {
        opt(&extParam)
    }
    return extParam
}

Then the caller looks like:

NewExtParam(value, WithSize(1), WithScale(2), WithPrec(3))

All of that aside, I would think your original proposal can be satisfied with a simple helper function like so.

func getOrDefault[T any](x []T, i int, d T) T {
	if i >= len(x) {
		return d
	}
	return x[i]
}

And then in your code sample it would be:

size := getOrDefault(opts, 0, nil)
scale := getOrDefault(opts, 1, nil)
prec := getOrDefault(opts, 2, nil)

@eaglebush
Copy link
Author

I feel like your new code looks so much shorter than your old code because you left out a bunch of lines that would have been repeated.

The new code would not be checking for the range of the array, but on the value itself:

if pr != nil {
  switch t := pr.(type) {
  case string:
	  if size64, err := strconv.ParseInt(t, 10, 32); err == nil {
		  prec = uint8(size64)
	  }
  case int:
	  prec = uint8(t)
  case uint8:
	  prec = t
  default:
	  prec= 0
  }
}

Also your new code implies that the values are being set to a pointer type, as you are comparing them to nil. Are they type *uint8?

I was assuming the pr variable would be nil if not passed, or a deliberate nil would be supplied by the user so I had to check nullity.

@eaglebush
Copy link
Author

Then the caller looks like:

NewExtParam(value, WithSize(1), WithScale(2), WithPrec(3))

This technique is awesome! I will definitely use it in my succeeding projects.

The ExtParam returns just as your defined struct type with a little difference:

type ExtParam struct {
	Value interface{}
	Size  int 
	Scale uint8
	Prec  uint8
}

I intend to use this a new data type for SQL query parameters, so I didn't think of having helper functions to sanitize user data. I only thought of a constructor for readability and ease of use.

All of that aside, I would think your original proposal can be satisfied with a simple helper function like so.

func getOrDefault[T any](x []T, i int, d T) T {
	if i >= len(x) {
		return d
	}
	return x[i]
}

And then in your code sample it would be:

size := getOrDefault(opts, 0, nil)
scale := getOrDefault(opts, 1, nil)
prec := getOrDefault(opts, 2, nil)

This is definitely helpful. About the proposal, this is my only use case yet.

@ianlancetaylor
Copy link
Contributor

Besides the discussion above, there seems to be an ambiguity here: if you have a slice of any, then it will be impossible to tell whether the value was present and nil, or whether the value was not present at all.

In general there are other ways to do this, and based on emoji voting there doesn't seem to be much enthusiasm for this approach. Therefore, this is a likely decline. Leaving open for four weeks for final comments.

@ianlancetaylor
Copy link
Contributor

No further comments.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Dec 7, 2022
@golang golang locked and limited conversation to collaborators Dec 7, 2023
@seankhliao seankhliao added the dotdotdot ... label Apr 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dotdotdot ... FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal Proposal-FinalCommentPeriod v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

6 participants