Added an alternative non-throw ex version of getting AppSetting

[oliversweb]

Was finding that I was try/catching in the case of cast failures so added silent option to where you supply a func<> to run in case of an exception either from convert() or missing key.

[davidwhitney]

Hey Billy

Totally see the need for this - but I'm not sure about this specific implementation.
It seems that adding a wrapper method might hide the intent when the key doesn't exist in the file - it's a very different kind of error than a conversion error - and using the Silent method provided, keys that were not found and keys that fail to convert / intercept / change type would seem identical, perhaps leading to confusing error messages.

Seems like we could have our cake and eat it though by simply extending the existing AppSetting method to look something like this:

     public string AppSetting(string key, Func<string> whenKeyNotFoundInsteadOfThrowingDefaultException = null, Func<T> handleConversionFailure = null)

Add adding a try catch around the intercept + convert block.

The only downside I see in this is that if you don't want to override the default configuration error exception, but DO want to override the conversion failure, the calling code will look a bit uglier with usages looking like

    cfg.AppSetting<TMything>("key", handlerConversionFailure: ()=>{ ... });

Maybe the middle road is to extend the existing method signature, and then add a specific overload that calls

    cfg.AppSetting<T>("key", null, conversionFunc);

Maybe something like

cfg.AppSettingConvert<T>("key", conversionErrorFunc);

To capture the meaning of "this is shorthand for catching conversion errors without losing the meaning of the error thrown when configuration settings are missing'.

Summary

I'm thinking:

 public T AppSetting<T>(string key, Func<T> whenKeyNotFoundInsteadOfThrowingDefaultException = null, Func<T> handleConversionFailure = null)
     {
            var rawSetting = Raw[key];

            if (rawSetting == null)
            {
                if (whenKeyNotFoundInsteadOfThrowingDefaultException != null)
                {
                    return whenKeyNotFoundInsteadOfThrowingDefaultException();
                }

                throw new ConfigurationErrorsException("Calling code requested setting named '" + key +
                                                       "' but it was not in the config file.");
            }

            try 
             {
                rawSetting = Intercept(key, rawSetting);
                return (T) Convert.ChangeType(rawSetting, typeof (T));
            }
            catch
            {
                if (handleConversionFailure != null)
                {
                    return handleConversionFailure ();
                }

                throw;
            }
        }


cfg.AppSettingConvert<T>(string key, Func<T> conversionErrorFunc = null){
   return AppSetting<T>(key, null, conversionErrorFunc);
}

As the useful middle ground. Thoughts?

[oliversweb]

Started down the path of extending AppSetting<> but was reluctant to dirty the signature with 2 optionals that in my case will be the same regardless of MissingKey or Conversion so one Func<>{Had some trouble so check your config, in the meantime using the default value} would fit all.

Of course I can work with the 'middle ground' but what about this as an alternative overload;

    public T AppSettingSilent<T>(string key, Func<T> insteadOfThrowingAnyException = null)
    {
        return AppSetting(key, insteadOfThrowingDefaultException, insteadOfThrowingDefaultException);
    }

[davidwhitney]

Sure, that makes sense to me.

[oliversweb]

I added both overloads; AppSettingConvert() & AppSettingSilent(), for a little more completeness.

[oliversweb]

@davidwhitney nudge :)

[davidwhitney]

I'm in Sicily... Let's see if my automation around this works ;)

[davidwhitney]

Right, I've merged into the published branch, so if all is well, this'll be on nuget momentarily. But I'm on my phone in Sicily, so let me know if it fucks up ;)

[oliversweb]

:) ty ... have a great time

On Fri, 29 May 2015 16:48 David Whitney [email protected] wrote:

Right, I've merged into the published branch, so if all is well, this'll
be on nuget momentarily. But I'm on my phone in Sicily, so let me know if
it fucks up ;)

—
Reply to this email directly or view it on GitHub
#5 (comment)
.