Skip to content

proposal: encoding/json: Add method MustFloat64 and MustInt64 for json.Number #68642

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

Closed
MakDon opened this issue Jul 30, 2024 · 3 comments
Closed
Labels
Milestone

Comments

@MakDon
Copy link
Contributor

MakDon commented Jul 30, 2024

Proposal Details

It would be more convinced if we could have methods MustFloat64 and MustInt64 that wraps Float64 and Int64.

Why

Assume we need to copy some field from a json object to another struct:

type Src struct {
	Foo json.Number `json:"foo"`
}

type Dest struct {
	Foo int `json:"foo"`
	//Other fields
}

func loadSrc() Src {
	return Src{Foo: "100"}
}

func getDest() {
	src := loadSrc()
	tmpNum, _ := src.Foo.Int64()
	dest := &Dest{
		Foo: int(tmpNum),
		// Other fields
	}
	
}

A tmpNum is necessary to receive the returned value from Int64(). But if we have a MustInt64, we could do the same thing like:

func getDest() {
	src := loadSrc()
	dest := &Dest{
		Foo: int(src.Foo.MustInt64()),
		// Other fields
	}
}

If the dest has multi fields that comes from different json.Number, we need to declare multi tmp variables for each field.

And such expression is not allowed because Dest.Foo is int but Int64() returns a Int64

dest.Foo, _ = src.Foo.Int64()

With MustInt64, this expression is valid:

dest.Foo = int(src.Foo.MustInt64())

How

A simple wrapper for Float64 and Int64:

func (n Number) MustFloat64() float64 {
	f, err := strconv.ParseFloat(string(n), 64)
	if err != nil {
		panic(err)
	}
	return f
}

func (n Number) MustInt64() int64 {
	i, err := strconv.ParseInt(string(n), 10, 64)
	if err != nil {
		panic(err)
	}
	return i
}

Question

  • If we should panic if this field is empty string?
  • panic or return 0 when ParseInt/ParseFloat failed?
@gopherbot gopherbot added this to the Proposal milestone Jul 30, 2024
@gabyhelp
Copy link

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@AlexanderYastrebov
Copy link
Contributor

See related #54297 that proposes generic helper.

@seankhliao
Copy link
Member

I think this should fold into #54297
Panic is not a general purpose error handler, and the existing must variants are intended for global values that the programmer knows cannot fail. User provided json does not fall into that category.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants