code review 6338067: encoding/json: Marshal a map with numeric keys. (issue 6338067)

[mike...@gmail.com]

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golan...@googlegroups.com (cc: golang...@googlegroups.com,
golan...@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go


Description:
encoding/json: Marshal a map with numeric keys.

Json package only accept a string keys map to encode currently.
There will be an error when encoding a map with numeric keys.
Eg. [http://play.golang.org/p/11oo0H0NtE]
got "json: unsupported type: map[int]string"

In developing, it is common that using a map with numeric keys.
Then we must copy & convert numbers into strings befor encoding into
json.
It is not convenient.

This patch was created for encoding the map with numeric keys into json.

Sorry for my English. Hope I have explained it clearly.

Please review this at http://codereview.appspot.com/6338067/

Affected files:
M src/pkg/encoding/json/encode.go
M src/pkg/encoding/json/encode_test.go


Index: src/pkg/encoding/json/encode.go
===================================================================
--- a/src/pkg/encoding/json/encode.go
+++ b/src/pkg/encoding/json/encode.go
@@ -350,7 +350,11 @@
e.WriteByte('}')

case reflect.Map:
- if v.Type().Key().Kind() != reflect.String {
+ k := v.Type().Key().Kind()
+ if k != reflect.String && k != reflect.Uint && k != reflect.Uint16 &&
+ k != reflect.Uint32 && k != reflect.Uint64 &&
+ k != reflect.Uint8 && k != reflect.Int && k != reflect.Int16 &&
+ k != reflect.Int32 && k != reflect.Int64 && k != reflect.Int8 {
e.error(&UnsupportedTypeError{v.Type()})
}
if v.IsNil() {
@@ -364,7 +368,7 @@
if i > 0 {
e.WriteByte(',')
}
- e.string(k.String())
+ e.string(sv.get(i))
e.WriteByte(':')
e.reflectValue(v.MapIndex(k))
}
@@ -447,7 +451,19 @@
func (sv stringValues) Len() int { return len(sv) }
func (sv stringValues) Swap(i, j int) { sv[i], sv[j] = sv[j], sv[i] }
func (sv stringValues) Less(i, j int) bool { return sv.get(i) < sv.get(j) }
-func (sv stringValues) get(i int) string { return sv[i].String() }
+func (sv stringValues) get(i int) (r string) {
+ switch sv[i].Kind() {
+ case reflect.String:
+ r = sv[i].String()
+ case reflect.Uint, reflect.Uint16, reflect.Uint32,
+ reflect.Uint64, reflect.Uint8:
+ r = strconv.Itoa(int(sv[i].Uint()))
+ case reflect.Int, reflect.Int16, reflect.Int32,
+ reflect.Int64, reflect.Int8:
+ r = strconv.Itoa(int(sv[i].Int()))
+ }
+ return
+}

func (e *encodeState) string(s string) (int, error) {
len0 := e.Len()
Index: src/pkg/encoding/json/encode_test.go
===================================================================
--- a/src/pkg/encoding/json/encode_test.go
+++ b/src/pkg/encoding/json/encode_test.go
@@ -26,6 +26,19 @@
Mo map[string]interface{} `json:",omitempty"`
}

+var mapNumIndexExpected = `{"0":0,"2":"","4":null}`
+
+func TestMapNumberIndex(t *testing.T) {
+ m := map[int]interface{}{0: 0, 2: "", 4: nil}
+ got, err := Marshal(m)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got := string(got); got != mapNumIndexExpected {
+ t.Errorf(" got: %s\nwant: %s\n", got, mapNumIndexExpected)
+ }
+}
+
var optionalsExpected = `{
"sr": "",
"omitempty": 0,

[Monnand]

就在中文组里说一下吧。

这个特性应该和json标准不统一吧。json标准里好像规定key只能是字符串：http://www.json.org/

如果加入这个特性，最好是单独放在一个package里。比如叫ejson(extended json)之类的。

-Monnand

--
官网: http://golang-china.org/
IRC:  irc.freenode.net     #golang-china
@golangchina

[Xing Xing]

于 Tue, 26 Jun 2012 09:52:16 -0700
Monnand <mon...@gmail.com> 写道:

> 就在中文组里说一下吧。
>
> 这个特性应该和json标准不统一吧。json标准里好像规定key只能是字符串：http://www.json.org/

实际上，编码后的 json 串是与标准一致的 {"0":0, "2": "",
"4":null}。只是为了方便的一个补丁。因为在很多时候，map[int]interface{}如果要
json 化，就必须 copy 一个
map[string]interface{}。那成本上来将，只扫描一遍 map 要划算得多。

>
> 如果加入这个特性，最好是单独放在一个package里。比如叫ejson(extended
> json)之类的。
>
> -Monnand
>
> On Tue, Jun 26, 2012 at 7:52 AM, <mike...@gmail.com> wrote:
>
> > Reviewers: golang-dev_googlegroups.com,
> >
> > Message:
> > Hello golan...@googlegroups.com (cc:
> > golang...@googlegroups.com, golan...@googlegroups.com),
> >
> > I'd like you to review this change to
> > https://code.google.com/p/go
> >
> >
> > Description:
> > encoding/json: Marshal a map with numeric keys.
> >
> > Json package only accept a string keys map to encode currently.
> > There will be an error when encoding a map with numeric keys.
> > Eg.

> > [http://play.golang.org/p/**11oo0H0NtE<http://play.golang.org/p/11oo0H0NtE>

> > ] got "json: unsupported type: map[int]string"
> >
> > In developing, it is common that using a map with numeric keys.
> > Then we must copy & convert numbers into strings befor encoding into
> > json.
> > It is not convenient.
> >
> > This patch was created for encoding the map with numeric keys into
> > json.
> >
> > Sorry for my English. Hope I have explained it clearly.
> >
> > Please review this at

> > http://codereview.appspot.com/**6338067/<http://codereview.appspot.com/6338067/>
> >
> > Affected files:
> > M src/pkg/encoding/json/encode.**go
> > M src/pkg/encoding/json/encode_**test.go
> >
> >
> > Index: src/pkg/encoding/json/encode.**go
> > ==============================**==============================**=======
> > --- a/src/pkg/encoding/json/**encode.go
> > +++ b/src/pkg/encoding/json/**encode.go

> > @@ -350,7 +350,11 @@
> > e.WriteByte('}')
> >
> > case reflect.Map:
> > - if v.Type().Key().Kind() != reflect.String {
> > + k := v.Type().Key().Kind()
> > + if k != reflect.String && k != reflect.Uint && k !=
> > reflect.Uint16 &&
> > + k != reflect.Uint32 && k != reflect.Uint64
> > &&
> > + k != reflect.Uint8 && k != reflect.Int &&
> > k != reflect.Int16 &&
> > + k != reflect.Int32 && k != reflect.Int64 &&
> > k != reflect.Int8 {

> > e.error(&UnsupportedTypeError{**v.Type()})

> > }
> > if v.IsNil() {
> > @@ -364,7 +368,7 @@
> > if i > 0 {
> > e.WriteByte(',')
> > }
> > - e.string(k.String())
> > + e.string(sv.get(i))
> > e.WriteByte(':')
> > e.reflectValue(v.MapIndex(k))
> > }
> > @@ -447,7 +451,19 @@
> > func (sv stringValues) Len() int { return len(sv) }
> > func (sv stringValues) Swap(i, j int) { sv[i], sv[j] = sv[j],
> > sv[i] } func (sv stringValues) Less(i, j int) bool { return
> > sv.get(i) < sv.get(j) }
> > -func (sv stringValues) get(i int) string { return
> > sv[i].String() } +func (sv stringValues) get(i int) (r string) {
> > + switch sv[i].Kind() {
> > + case reflect.String:
> > + r = sv[i].String()
> > + case reflect.Uint, reflect.Uint16, reflect.Uint32,
> > + reflect.Uint64, reflect.Uint8:

> > + r = strconv.Itoa(int(sv[i].Uint())**)

> > + case reflect.Int, reflect.Int16, reflect.Int32,
> > + reflect.Int64, reflect.Int8:
> > + r = strconv.Itoa(int(sv[i].Int()))
> > + }
> > + return
> > +}
> >
> > func (e *encodeState) string(s string) (int, error) {
> > len0 := e.Len()

> > Index: src/pkg/encoding/json/encode_**test.go
> > ==============================**==============================**=======
> > --- a/src/pkg/encoding/json/**encode_test.go
> > +++ b/src/pkg/encoding/json/**encode_test.go

[Monnand]

原来如此。之前没有理解。个人觉得还是挺不错的特性。就看go team买不买帐了。

[lihui]

貌似有歧义。

{"0", ooxx} 的反序列化你准备生成什么？

map[string]interface{} 还是map[int]interface{}

[Xing Xing]

于 Wed, 27 Jun 2012 10:52:55 +0800
lihui <ustc....@gmail.com> 写道:

> 貌似有歧义。
>
> {"0", ooxx} 的反序列化你准备生成什么？
>
> map[string]interface{} 还是map[int]interface{}

仍然是 map[string]interface{}。

实际上，最初我是想实现 map[obj]interface{}，只要 obj 实现了 String()
方法，就能够 json 化。不过涉及的改动会比较大，所以先实现一部分，看看 Go team
怎么看这个问题，如果有必要再继续改进。否则，就得在自己的项目里通过某种补丁方式来解决这个便利性问题了。

[Leo Jay]

On Wed, Jun 27, 2012 at 9:18 AM, Xing Xing <mike...@gmail.com> wrote:
>

> 于 Tue, 26 Jun 2012 09:52:16 -0700
> Monnand <mon...@gmail.com> 写道:
>
> > 就在中文组里说一下吧。
> >
> > 这个特性应该和json标准不统一吧。json标准里好像规定key只能是字符串：http://www.json.org/
>
> 实际上，编码后的 json 串是与标准一致的 {"0":0, "2": "",
> "4":null}。只是为了方便的一个补丁。因为在很多时候，map[int]interface{}如果要
> json 化，就必须 copy 一个
> map[string]interface{}。那成本上来将，只扫描一遍 map 要划算得多。
>

恕我直言，为了方便，在标准库里加入非标准类型的支持，如果我是go team member，我不会接受这样的patch。

而你所需要的，仅仅是这样的一个工具函数：
func ConvertToStringMap(m map[int]string) map[string]string {
result := map[string]string{}
for k, v := range m {
result[strconv.Itoa(k)] = v
}
return result
}

你对成本的计算，我认为十之八九是瞎操心，这里不大可能成为你程序的瓶颈。
Premature optimization is the root of all evil.

--
Best Regards,
Leo Jay

[Xing Xing]

于 Wed, 27 Jun 2012 13:14:00 +0800
Leo Jay <python...@gmail.com> 写道:

> 恕我直言，为了方便，在标准库里加入非标准类型的支持，如果我是go team
> member，我不会接受这样的patch。

go team 不接受这个 patch 的确是更加可能的情况。

>
> 而你所需要的，仅仅是这样的一个工具函数：
> func ConvertToStringMap(m map[int]string) map[string]string {

这确实是我现在正在使用的方法。但是对于一个嵌套多层的 struct 和
map，十分不便。

>
> 你对成本的计算，我认为十之八九是瞎操心，这里不大可能成为你程序的瓶颈。
> Premature optimization is the root of all evil.

对于我来说，这个 patch 的主要目的，就是提供合理的便利性。
成本的事情，只是一个预期，并没有测量依据，所以不要当真。

[lihui]

map[int]T也不算是不标准了，这类东西不支持似乎也让人感觉有些奇怪。

不过对于key非string的map，个人更倾向于序列化为[{key: xxx,value:yyy}*]的形式。或许正是由于这种偏好的不确定性，标准库才不处理，留待用户自行决定。

[lihui]

对于多层嵌套的情况，  可以type 一下给出业务相关的名称，自定义Marshal  接口，然后在嵌套中使用这个新的类型。

2012/6/27 Xing Xing <mike...@gmail.com>

[minux]

On Wed, Jun 27, 2012 at 11:32 AM, Xing Xing <mike...@gmail.com> wrote:

于 Wed, 27 Jun 2012 10:52:55 +0800
lihui <ustc....@gmail.com> 写道:

> 貌似有歧义。
>
> {"0", ooxx} 的反序列化你准备生成什么？
>
> map[string]interface{} 还是map[int]interface{}

仍然是 map[string]interface{}。

实际上，最初我是想实现 map[obj]interface{}，只要 obj 实现了 String()
方法，就能够 json 化。不过涉及的改动会比较大，所以先实现一部分，看看 Go team
怎么看这个问题，如果有必要再继续改进。否则，就得在自己的项目里通过某种补丁方式来解决这个便利性问题了。

其实我建议这种问题先去golang-dev讨论下会比直接提交CL好点，

尤其是本CL被assign给最近两周都不能上线的rsc了，因为如果通过gocodereview给assign了reviewer

那么基本意味着别人不理了。最好还是先去golang-dev说下，先看看大家的反响再发CL。