Possible performance issues with C# code in a legacy project
<p>I started working recently in a new project where we have thousands of lines of legacy code. We are facing several performance issues. I decided to take a look at the code and saw the following. There's a class:</p>

<pre><code>public class BaseDataAccess
{
    private Database dB;

    public Database DB
    {
        get
        {
            if (dB == null)
            {
                dB = DatabaseFactory.CreateDatabase();
            }
            return dB;
        }
    }
}
</code></pre>

<p>And many descendant classes which inherit from the previous base class. Internally, those other classes make use of the DB property, like this:</p>

<pre><code>DataSet ds = DB.ExecuteDataSet(spGetCustomersSortedByAge);
</code></pre>

<p>Finally, there's a huge class (5000 lines of code) with tens of methods like the following:</p>

<pre><code>public void ProcessPayments()
    {
        try
        {
            List&lt;Employee&gt; employees = new EmployeesDA().SelectAll(null);
            foreach (Employee employee in employees)
            {

                employee.Account = new MovementsDA().SelectAll(employee.Id, DateTime.Now);

                ...

                City city = new CitiesDA().Select(zone.cityId);

                ...

                Management m = new ManagmentDA().Select(city.id);

            }
        }
        catch (Exception ex)
        {
                ...
        }

    }
</code></pre>

<p>Note in the previous method EmployeesDA, MovementsDA, CitiesDA and ManagmentDA all are inheritors of BaseDataAccess and internally use their respective DB properties. Also note they are constantly being instantiated inside foreach loops (many times within 2 levels of nesting).</p>

<p>I think the instantiation itself is suspicious but I'm more concerned about what's going on with the database connections here? Is every DA instantiated opening a new underlying connection? How bad is this code? </p>

<p>As a side note about the solution I was considering in case this code should be fixed: I was considering making every constructor private so the compiler starts complaining about the instantiations and refactor the instantiations with calls to the GetInstance method (singleton pattern) to avoid the recreation of the objects and underlying connections. But, I'm not sure if this could also be dangerous in some way, for example, if the connections may get closed. The current code doesn't have that problem because of the instantiatons happening all the time.</p>