trjhtr

I have a class that loads locales from a database. I'm not sure if locale is the right word. It's basically loading texts for a game, generic sayings like alerts and error messages.

I'm also not sure if it should be called WhateverHandler or WhateverController, maybe even WhateverManager. I tend to get really OCD about my code, so some advice would be nice.

public class LocaleHandler

 private static readonly ILogger Logger = new ConsoleLogger(typeof(LanguageLocale));

 private readonly ConcurrentDictionary<string, string> _localeKeyValue;

 public LocaleHandler()
 
 using (var dbConnection = PlusEnvironment.GetDatabaseManager().Connection)
 
 _localeKeyValue = FetchLocale(dbConnection);
 

 Logger.Trace($"Fetched _localeKeyValue.Count language locales from the database.");
 

 private static ConcurrentDictionary<string, string> FetchLocale(DatabaseConnection dbConnection)
 
 dbConnection.SetQuery("SELECT * FROM `server_locale`");

 using (var reader = dbConnection.ExecuteReader())
 
 if (!reader.HasRows)
 
 return new ConcurrentDictionary<string, string>();
 

 var locale = new ConcurrentDictionary<string, string>();

 while (reader.Read())
 
 locale.TryAdd(reader.GetString("key"), reader.GetString("value"));
 

 return locale;
 
 

 public bool TryGetLocale(string key, out string value)
 
 return _localeKeyValue.TryGetValue(key, out value);
 

First, since you are concerned about naming: this class doesn't really handle, manage, or control anything. It does two things: queries a database, and makes those results available via a TryGet interface. In fact, you could split it into two classes, LocaleLoader and Locale. Though I wonder if "locale" is even the correct term since there is no locale supplied (like "en-us"), it's more of a string table. On the topic of naming, "_localeKeyValue" is not very good; it describes the implementation and not the purpose. Something like "Strings" would be better. Even "_localeDict" would be an improvement.

As noted in the comments, the ConcurrentDictionary is unnecessary; Dictionary supports concurrent readers as long as there are no writers. In your case, all reading occurs after all writing.

@paparazzo is right about checking for no results being unnecessary. The result is the same, it's no faster, and it's more code.

Consider allowing the constructor to be passed in a database connection to use. This makes it more flexible and removes a dependency.

TryGetLocale: are you really getting a locale? I think you are getting a string, translation, or localization. Without seeing how this code is used, I question the usefulness of the TryGet-style interface. For (what I imagine to be) my similar code, I like to just return the key if it's not found. Something like:

public String GetString(String key)

 String value;
 return _localeKeyValue.TryGetValue(key, out value) ? value : key;

This means that when the code encounters an untranslated string it's obvious to the devs and testers what is missing. The other possibility would be to throw an exception when you can't find the key, but on the off-chance that you ship software with a missing string it's better for the user to see a message box like "IMG_CONVERSION_JPEG_SUCCESS" than "Illegal argument: key".

This is not required

if (!reader.HasRows)

 return new ConcurrentDictionary<string, string>();