-1
\$\begingroup\$

I am writing a C# Database connection class and I am trying to adhere to best practices and produce clean code.

In the CreateAndOpenDatabaseConnection function below I am instantiating the connection object and opening the connection.

Is this a violation of the Single Responsibility Principle (SRP), because of me instantiating and opening the connection in one function?

The Single Responsibility Principle (SRP) is the concept that any single object in object-oriented programing (OOP) should be made for one specific action

    public static class DatabaseConnector
    {
        public static SqlConnection CreateAndOpenDatabaseConnection()
        {
            string connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
            SqlConnection connection = new SqlConnection(connectionString);
            connection.Open();
            return connection;
        }         

//other class functions

        public static List<PlaceHolder> GetCurrentPlaceHolder()
        {
            var PlaceHolderList = new List<PlaceHolder>();

            using (SqlConnection connection = CreateAndOpenDatabaseConnection())
            {
                string query = {query};

                using (SqlDataReader reader = CreateDataReader(CreateSqlCommand(query, connection)))
                {
                    while (reader.Read())
                    {
                        PlaceHolderList.Add(new PlaceHolder{
                            ...,
                        });
                    }
                }
            }
            return PlaceHolderList;
        }                
    }

Should I write one function that instantiates the connection and one that only opens the connection like so:

        public static SqlConnection CreateDatabaseConnection()
        {
            string connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
            SqlConnection connection = new SqlConnection(connectionString);    
            return connection;
        }

        public static SqlConnection OpenDatabaseConnection()
        {
            SqlConnection connection = CreateDatabaseConnection();
            connection.Open();
            return connection;
        }
        

or is it overkill and they are one thing and belong in one function?

\$\endgroup\$
7
  • 3
    \$\begingroup\$ Can you clearly state the SRP? Can you clearly say why your function might violate the SRP? And can you say why you think the SRP is a principle worth paying any attention to in the first place in the context of this coding task? \$\endgroup\$ Commented Mar 1 at 21:43
  • \$\begingroup\$ Create a connection, command, and reader right on the spot by wrapping them in using. You will be surprised that the number of rows remains the same. And only the connection string needs to be received from the outside: inject it into the class constructor and store it in the field. \$\endgroup\$ Commented Mar 2 at 16:14
  • \$\begingroup\$ Do not write your own ORM. Use EF or Dapper. \$\endgroup\$
    – BCdotWEB
    Commented Mar 3 at 19:53
  • 2
    \$\begingroup\$ OK, so you've written two versions of your function. Is either of them a "module"? You've stated that the SRP applies to "modules". If these are not modules, then the SRP doesn't apply and the question doesn't make sense. If yes, they are modules, then how many "actors" is the first version "responsible" to? How many "actors" is the second version "responsible" to? \$\endgroup\$ Commented Mar 4 at 19:37
  • 1
    \$\begingroup\$ Finally: the SRP is about robustly designing for change. What changes do you anticipate will be made to this code, and who will be requesting the change? Without knowing the answer to that question, it makes no sense to talk about the SRP, since the SRP is specifically about anticipating future needs. \$\endgroup\$ Commented Mar 5 at 23:39

1 Answer 1

1
\$\begingroup\$

It depends on what you deem as an action. If the action is "get me a usable database connection" and if this action consists of a three-step process, there shouldn't be anything wrong with doing these steps in one function. Also, is there ever a case where you need to do these steps individually?

As far as I know, the goal of the Single Responsibility Principle is to avoid overly complex code with many different things going on. Not only is complex code hard to understand by other people, including your future self, but it's also hard to test and to find and fix defects. If you apply this thought to your example, then there's nothing wrong with creating the SQL connection string, SQL connection object, database connection object and opening said connection in one code block because this code is very easy to understand.

In my daily work, I actually seldom think about such concepts. Instead, I believe it's easier to think about code in terms of: "Is it easy to read, understand, maintain, and test?" -- that's the core of many good coding practices.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Yes the goal of the SRP is increasing robustness by decreasing "number of things going on", but its motivation was not specifically making it easier to find/fix defects. Rather, to prevent the introduction of new defects. Code that is responsible to two constituencies will change when either changes their requirements, but a change to benefit one might break the other. \$\endgroup\$ Commented Mar 5 at 23:34
  • 1
    \$\begingroup\$ As you correctly note, there's almost never a reason to actually think about this in real world code because we typically don't write classes that are "responsible" to multiple "actors" in the first place. I've written code professionally for 30 years and never once thought about the SRP specifically; rather, as you say, I think about correctness, readability, testability, maintainability, extensibility, and so on. \$\endgroup\$ Commented Mar 5 at 23:36
  • 1
    \$\begingroup\$ Well, in my comparatively short career, I've already seen some "god objects" and probably implemented some myself in my early years. That is to say, it does happen. But as soon as this falls onto your feet, you start to learn how to do it better, that's why I'm also a proponent of the "dare to do mistakes" philosophy. \$\endgroup\$ Commented Mar 8 at 1:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.