EvilDr EvilDr - 1 month ago 4
ASP.NET (C#) Question

Do these static application properties need to be locked?

In my web application, I read some settings from an external config file during

Application_Start
, then access them across the application's many methods:

namespace Common
{
public static class CommonConfigSettings
{
public static string DataSource { get; set; }
public static string DatabaseName { get; set; }
public static string DatabaseUserName { get; set; }
public static string DatabasePassword { get; set; }
}
}


During
Application_Start
these are read from an XML file into the static variable:

DataSource = els.FirstOrDefault(item => item.Attribute("key").Value == "DataSource").Attribute("value").Value;
DatabaseName = els.FirstOrDefault(item => item.Attribute("key").Value == "DatabaseName").Attribute("value").Value;
DatabaseUserName = els.FirstOrDefault(item => item.Attribute("key").Value == "DatabaseUserName").Attribute("value").Value;
DatabasePassword = els.FirstOrDefault(item => item.Attribute("key").Value == "DatabasePassword").Attribute("value").Value;


In the application they are used as follows:

myConn.ConnectionString = string.Format("Persist Security Info=False; User ID={0}; Password={1}; Initial Catalog={2}; Data Source={3}; Connection Timeout=60",
CommonConfigSettings.DatabaseUserName,
CommonConfigSettings.DatabasePassword,
CommonConfigSettings.DatabaseName,
CommonConfigSettings.DataSource);


At no point in time are the static values written to following
Application_Start
- they are only read back out (although perhaps by 2+ people simultaneously). There are also no static methods, just properties. I read about locking and thread safety here and here, but have only confused myself. Should I implement locking on these values, and if so, at what point please?

Answer

If you are absolutely sure that those properties are written just once (and prior to all read operations), there is no need for locking.

EDIT: The question is: Will it always be so? If you would come to the need to replace this database access information at runtime, you would run into the problem of non-atomic operations (e.g. reading a new database username and and old password if the writing thread would be interrupted at the right/"wrong" time). May be it would be good to provide a method to return all needed data in a single struct. This method can be provided with a thread locking mechanism in the future if the need would arise...

public struct DatabaseAccessData
{
    public string DataSource { get; set; }
    public string DatabaseName { get; set; }
    public string DatabaseUserName { get; set; }
    public string DatabasePassword { get; set; }
}

public static class CommonConfigSettings
{        
    private static string DataSource { get; set; }
    private static string DatabaseName { get; set; }
    private static string DatabaseUserName { get; set; }
    private static string DatabasePassword { get; set; }

    public static void SetDatabaseAccessData(DatabaseAccessData data)
    {
        DataSource = data.DataSource;
        DatabaseName = data.DatabaseName;
        DatabaseUserName = data.DatabaseUserName;
        DatabasePassword = data.DatabasePassword;
    }

    public static DatabaseAccessData GetDatabaseAccessData()
    {
        return new DatabaseAccessData
        {
            DataSource = DataSource,
            DatabaseName = DatabaseName,
            DatabaseUserName = DatabaseUserName,
            DatabasePassword = DatabasePassword
        };
    }

Let me say that i am not a fan of "static" in this case. If some of your classes depend on having the common configuration settings, you should pass an instance of CommonConfigSettings to them via constructor parameter or property (see "Dependency injection"). I prefer the first, as it is more rigid / strict; you cannot forget to pass an important dependency then.