Feature: Add `case interface{}:` to `ConvertMapI2MapS` about dyno HOT 5 CLOSED

I think accepting structs for conversion is out of scope of this library (at least for now).

But that doesn't mean you can't do what you want. You don't want to convert the whole Config struct just its Options field. So do that: pass only the Config.Options field to ConvertMapI2MapS() function and assign its return value and you're done (you need to use type assertion on the result):

var config *Config
// ...load the config

// And now convert the Config.Options field
// to make sure it only contains maps with string keys:

config.Options = dyno.ConvertMapI2MapS(config.Options).(map[string]interface{})

(Note than in this case it would work even without assigning the return value back to config.Options because what we pass is of static type map[string]interface{}, but in general this is the proper usage.)

About the type alias:

type Options map[string]interface{}

This is not a type alias, this is a type definition, that's why you need to use explicit type conversion. This would be a type alias:

type Options = map[string]interface{}

Read the difference in Spec: Type declarations. This is available since Go 1.9, and passing a value of type Options is identical to passing a value of type map[string]interface{}, so no explicit conversion would be needed here.

from dyno.

Hello @icza , Thanks for the quick reply. I tried passing in the object of type Options directly, but it does not work, it only works when I explicitly cast it, as I've shown in the example. It seems to be that the case statement does not detect it as map[string]interface{}, but instead shows the type of config.Options. I've read the specification, and it does indeed say that this is exactly how things should work. Type would be config.Options, but Kind would be map

from dyno.

So may the issue be closed?

from dyno.

I was hoping that I can avoid doing the external casting at the very least, so that this type of determination is done inside the function. Since the signature of ConvertMapI2MapS says it just wants interface{}, it's hard to know what exactly it wants inside, so from API transparency I would suggest internally handling the case when a passed object is of a custom type but of a map kind.

Something along the lines of:

case interface{}:
		val := reflect.ValueOf(x)
		switch val.Kind() {
		case reflect.Map:
			t := val.Type()
			// Create the map of the specific type. Key type is t.Key(), and element type is it
			m := reflect.MakeMap(reflect.MapOf(t.Key(), t.Elem()))

			// Copy values to new map
			for _, mk := range val.MapKeys() {
				m.SetMapIndex(mk, val.MapIndex(mk))
			}

			v = ConvertMapI2MapS(m.Interface())

from dyno.

The package doc:

Primary goal is to easily handle dynamic objects and arrays (and a mixture of these) that are the result of unmarshaling a JSON or YAML text into an interface{} for example.

Unmarshaling JSON or YAML into a value of interface{} type will never result in a custom map type, only maps being of type map[string]interface{} or map[string]interface{}.

Doc of ConvertMapI2Maps() also states:

Recursion is implemented into values of the following types:
-map[interface{}]interface{}
-map[string]interface{}
-[]interface{}

So I think it's clear enough what you may pass (or rather what will be processed).

If you have a value of custom map type, you can still use this library by explicitly converting your value.

Current implementation does not use reflection at all, which results in rather good performance (reflection is much slower than type assertions and type switches).

That being said, I still don't think this addition is justified.

from dyno.