How to initialize base class property once instead of passing it from all derived classes?

Question

I have a small library where I am performing 1 long running operation and based on that operation,I am saving some data inside database tables.Now this is small library which does not include alot of database tables operation hence I have used Ado.net to manage data access layer.

I have created base class where I have put Connection string and Ado.net commands execution.

I have 3-4 class with some methods performing insert,update and delete but the problem is I have to pass connection string to each of this 4 class whenever I am creating the object of this class.

Code :

 internal abstract class BaseRepo
    {
        private readonly string connectionString;

        protected int TestId;
        protected int VariantId;


        public BaseRepo() { }

        protected BaseRepo(string connection)
        {
            this.connectionString = connection;
        }

        internal virtual void ExecuteQuery(string query,
            IList<SqlParameter> parameters)
        {
            using (SqlConnection conn = new SqlConnection(connectionString))
            {
                using (SqlCommand command = new SqlCommand(query, conn))
                {
                    conn.Open();
                    foreach (SqlParameter parameter in parameters)
                    {
                        command.Parameters.Add(parameter);
                    }
                    command.ExecuteNonQuery();
                }
            }
        }

        internal virtual void ExecuteQueryWithTransaction(SqlConnection connection,SqlTransaction transaction, string query,
            IList<SqlParameter> parameters)
        {
            using (SqlCommand command = new SqlCommand(query, connection, transaction))
            {
                foreach (SqlParameter parameter in parameters)
                {
                    command.Parameters.Add(parameter);
                }
                command.ExecuteNonQuery();
            }
        }

        internal virtual int ExecuteScalar(string query,
            IList<SqlParameter> parameters)
        {
            using (SqlConnection conn = new SqlConnection(connectionString))
            {
                using (SqlCommand command = new SqlCommand(query, conn))
                {
                    conn.Open();
                    foreach (SqlParameter parameter in parameters)
                    {
                        command.Parameters.Add(parameter);
                    }
                    var data = command.ExecuteScalar();
                    if (data == null)
                        return 0;
                    return (int)data;
                }
            }
        }
    }


internal class VariantRepo : BaseRepo
    {
        public VariantRepo(string connectionString,int testId,int variantId) : base(connectionString)
        {
            TestId = testId;
            VariantId = variantId;
        }

        public VariantRepo(testId) : base(connectionString)
        {
           TestId = testId;
        }
        public void DeleteVariantData()
        {
            string query = "";
            List<SqlParameter> parameters = new List<SqlParameter>();
            parameters.Add(new SqlParameter("@TestId", TestId));
            parameters.Add(new SqlParameter("@VariantId", VariantId));
            ExecuteQuery(query, parameters);
        }
    }

internal class RegionRepo : BaseRepo
    {
        public RegionRepo(string connectionString, int variantId) : base(connectionString)
        {
            VariantId = variantId;
        }
        public int GetRegionIdByVariantId()
        {
            string query = "";
            List<SqlParameter> parameters = new List<SqlParameter>();
            parameters.Add(new SqlParameter("@id", VariantId));
            return ExecuteScalar(query, parameters);
        }
    }

As you can see I have 4 class here and from each of this class I have to pass connection string to base class :

1) VariantRepo

2) RegionRepo

3) CategoryRepo(not shown but it is same like above 2)

4) TestRepo(not shown but it is same like above 2).

So there are 2 things that I would like to address here :

1) It is possible to design base class in a way that I have to pass connection string only once and not for each of the 4 concrete classes?

2) I want to hide this below dirty logic(Sql parameter creation code) behind some class to have better readability because I have to do this at so many places:

List<SqlParameter> parameters = new List<SqlParameter>();
parameters.Add(new SqlParameter("@TestId", TestId));
parameters.Add(new SqlParameter("@VariantId", VariantId));

I thought to have base class as concrete class but than having base class as abstract class makes more sense I guess that is why I have marked base class as abstract.

Can someone please help me to design this in better way?

You would be a lot happier using [Dapper](https://github.com/StackExchange/Dapper) instead of ADO. It has a much cleaner way of passing parameters. — Robert Harvey, Jul 04 '19 at 15:24

R. Schmitz · Answer 1 · 2019-07-04T13:32:38.557

This is going to be a frame challenge answer.

It is possible to design base class in a way that I have to pass connection string only once and not for each of the 4 concrete classes?

You shouldn't try to achieve what you want to with a base class.

Why not?

Basically, if you use a base class for this, it might work fine now. Over time, your code will get more and more complex. The older code is, the more difficult it is to change it. And then when at some point one of your repos needs a different connection string, you will have to untangle everything and it. will. be. PAIN.

So having to provide the connection string to each repo is actually a good thing. In the end. At the moment it seems like just more work, but it's worth it going forward.

What instead?

This is a situation where favoring composition over inheritance is actually easier then messing around with inheritance in a way it isn't really supposed to be used. "Switching over" won't actually be a lot of work for you, because you already have the common functionality in your base class. In order to adjust it just

give it a more fitting name, RepoConnector or something like that
make it non-abstract
make the constructor public or internal

Then you can inject it into your repos, store it in a field and use it from there. For example, your RegionRepo would look something like this:

internal class RegionRepo
    {
        private readonly RepoConnector _repoConnector ;


        public RegionRepo(RepoConnector repoConnector, int variantId)
        {
            _repoConnector = repoConnector;
            VariantId = variantId;
        }


        public int GetRegionIdByVariantId()
        {
            string query = "";
            List<SqlParameter> parameters = new List<SqlParameter>();
            parameters.Add(new SqlParameter("@id", VariantId));
            return _repoConnector.ExecuteScalar(query, parameters);
        }
    }

Admittedly, this isn't exactly what you wanted. Now the connection string is only provided once, but instead the RepoConnector is being passed to all the repos.

However, passing the info to each repo is the "proper" way to do it that won't give you headaches later on. Also, you do still get "the best of both worlds": If you want to change the connection string, there's only one place to change now, PLUS you won't have to rewrite common functionality for each repo.

"Dirty logic"?

The same advice does not really apply for this bit:

List<SqlParameter> parameters = new List<SqlParameter>();
parameters.Add(new SqlParameter("@TestId", TestId));
parameters.Add(new SqlParameter("@VariantId", VariantId));

It's not that dirty. However, you say "I have to do this at so many places". If it's really bad enough, you might want to add a little helper method to the RepoConnector, something like this:

internal void ExecuteQuery(string query, string name1, int value1,
    string name2, int value2)
{
    List<SqlParameter> parameters = new List<SqlParameter>();
    parameters.Add(new SqlParameter(name1, value1));
    parameters.Add(new SqlParameter(name2, value2));
    ExecuteQuery(query, parameters);
}

As is probably obvious, this only helps if you're really doing the same thing (as in the example, "calling ExecuteQuery with 2 int parameters") multiple times. Another alternative might be using params:

void ExecuteQuery(string query, params SqlParameter[] parameters)
{
    // Everything else can stay the same here because foreach still works on arrays
}

// You can now call it this way:
ExecuteQuery(query, new SqlParameter("@TestId", TestId), new SqlParameter("@VariantId", VariantId));
// You can put as many "new SqlParameter()" as you want, or even without any:
ExecuteQuery(query);

But again, this doesn't give you a lot, so it's only worth it in some cases. If you're always doing slightly different things, it's probably easier to do that by writing slightly different code for each.

Thank you so much for such a detailed answer.I dont always have 2 parameters.Some methods have 2 and some have 3 or 4 like wise.So this will not help.any better alternative than this? — I Love Stackoverflow, Jul 04 '19 at 13:11
@ILoveStackoverflow No, not really. I added an alternative using `params`, but that's as flexible as it gets for this situation. But as I said, before, what you have at the moment is not that dirty - a method with 7 lines for 4 parameters is not that bad. — R. Schmitz, Jul 04 '19 at 13:36
@ILoveStackoverflow The issue is that if you do something repeatedly, you can put it in a common place, but if you actually need to do different things, you'll have to do that by writing different code each time. Only that what repeats can be "standardized", and passing a list of `SqlParameter` to the method is already a "standardized" way to deal with an arbitrary number of values. — R. Schmitz, Jul 04 '19 at 13:41

score 0 · Answer 2 · answered Jul 04 '19 at 11:26

It is possible to design base class in a way that I have to pass connection string only once and not for each of the 4 concrete classes?

Short answer - No.

If you're going to use inheritance and work with [instances of] subclasses, then you have to use the proper plumbing to allow the derived classes to work with the base class.

When you instantiate instances of the subclasses, .Net implicitly constructs the base class for you and, given the constructors you've got defined, that construction process requires the connection string, so you have to pass it from subclass to base class.

This is perfectly normal and nothing to be concerned about.

I want to hide this below dirty logic ...

I wouldn't say it was particularly "dirty".

It's clear, explicit in what it's doing, can be understood at a glance and is probably specific to each, individual, execution. Personally, I'm not seeing a problem with it.

Sure, you could replace it with some sort of "parameter builder", like this:

List<SqlParameter> parameters = new CustomSqlParameterBuilder() 
   .AddNameValue( "@TestId", TestId )
   .AddNameTypeValue( "@VariantId", GetType( System.Int32 ), VariantId )
   .Build();

... but is it that really any clearer / quicker / better? YMMV.

I am thinking something like this : private static readonly string ConnectionString = ConfigurationManager.ConnectionStrings["myconnectionstring"].ConnectionString; — I Love Stackoverflow, Jul 04 '19 at 11:29

How to initialize base class property once instead of passing it from all derived classes?

2 Answers2

Why not?

What instead?

"Dirty logic"?