Performance issues with repeated calls to ConfigurationManager.AppSettings to get appsetting values?

Question

I'm working on a code base that has a lot of identical ConfigurationManager.AppSetting calls scattered throughout.

Does this sound like a possible performance issue?

Or because the data being very small is trivial and not 'expensive'? Constantly going back to the file to get the data, or does the .NET runtime cache the file/values/calls?

If this isn't a performance issue is it just a disorganized approach to accessing the application configuration values, and should just be re-factored to be cleaner and consistent implementation of accessing the settings?

Answer 1

I would say it's more of the code maintainability issue than performance issue. A simple dictionary lookup on AppSettings isn't going to be a problem unless you have code that tries to perform lookup on AppSettings in a loop that runs say hundred times. Surely such a code will cause performance problem. But even more important is you will have ConfigurationManager.AppSettings["MyKey"] throughout your codebase. You are introducing a magic string. If you have to change the key in your configuration file, you will have to do a thorough search and replace in all your projects. Moreover, we usually make some decision based on the value stored in appSettings. It's not always straighforward read and use the value as-is. Sometimes you take decision based on the value. For ex,

if (ConfigurationManager.AppSettings["DebugMode"] == "yes")
  do this
else
  do that

You might be repeating this logic in hundred places. Now let's say you need to add another condition there:

if (ConfigurationManager.AppSettings["DebugMode"] == "yes" || ConfigurationManager.AppSettings["InternetNotAvailable"] == "yes")
  do this
else
  do that

This gets messy. Your code starts to stink.

So, I always recommend my dev team to never use ConfigurationManager.AppSettings anywhere in the code. Use some static class where you read the configuration values and all such decisions are precached into a single variable. For ex,

static class ConfigHelper
{
  private readonly static bool ExternalWebserviceCallAllowed = ConfiguationManager.AppSettings["DevMode"] == "false" && ConfigurationManager.AppSettings["InternetAvailable"] == "true";

}

.
.
if (ConfigHelper.ExternalWebserviceCallAllowed)
   do this
else
   do that

This is not only better in performance, but also highly maintainable and extensible code.

Answer 2

A few things here.

1. You can validate if this is a performance issue by using something like ANTS Profiler or DotTrace that allows you to inspect the performance of the application
2. I would say that you MIGHT be open for some issues in the future if you scatter calls all over the place, think for example if you decide to change the name of one of the app settings, it could be disastrous. I personally recommend centralizing this type of thing if for any reason to allow for future changes.
3. From a performance perspective, be careful to not "pre-optimize" something that is just not needed, you might end up adding more complexity when it just isn't needed.