Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

ConfigurationExtensions.Get<T>() should support TypeConverters #130

Closed
Kieranties opened this issue Jan 21, 2015 · 6 comments
Closed

ConfigurationExtensions.Get<T>() should support TypeConverters #130

Kieranties opened this issue Jan 21, 2015 · 6 comments

Comments

@Kieranties
Copy link

The current implementation uses Convert.ChangeType which only handles types that implement IConvertible.

A small change to allow the use of TypeConverters would improve mapping values to other types.

e.g.

#if NET45 || ASPNET50 || ASPNETCORE50
        public static T Get<T>(this IConfiguration configuration, string key)
        {
            var type = typeof(T);
            var value = configuration.Get(key);
#if ASPNETCORE50            
            return (T)Convert.ChangeType(value, type);
#elif NET45 || ASPNET50
            if (typeof(IConvertible).IsAssignableFrom(type))
            {
                return (T)Convert.ChangeType(value, type);
            }
            else
            {
                var converter = TypeDescriptor.GetConverter(type);
                if (converter == null) throw new InvalidCastException("Could not find a valid TypeConverter for type " + type.FullName);

                return (T)converter.ConvertFromInvariantString(value);
            }
#endif
        }
#endif
@Kieranties
Copy link
Author

Hey all, I guess with #199 in place, this can be closed?

Edit: Looks like the ConfigurationBinder has been moved into this project from Options. So the above still applies (but in the ConfigurationBinder class)

@divega
Copy link

divega commented May 20, 2015

POCO binding was moved from Options into the Configuration repo recently (see #205). It is currently in Microsoft.Framework.ConfigurationModel although I think it is misplaced there (see #210). Regardless of the specific location, would any part of this improvement apply to the new code?

@Kieranties
Copy link
Author

@divega I'd think the logic when using the full runtime could still be applied at https://github.com/aspnet/Configuration/blob/dev/src/Microsoft.Framework.ConfigurationModel/ConfigurationBinder.cs#L217. Essentially, anywhere Convert.ChangeType is used TypeConverters should also be considered.

@HaoK
Copy link
Member

HaoK commented Aug 27, 2015

This has been done

@HaoK HaoK closed this as completed Aug 27, 2015
@HaoK
Copy link
Member

HaoK commented Aug 27, 2015

3476fad

@Kieranties
Copy link
Author

Brilliant!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants