2

I'm using the following class to wrap my CRUD operations:

public class DAL
    {
        private MySqlConnection conn;
        private MySqlCommand cmd = new MySqlCommand();
        private MySqlDataAdapter adap = new MySqlDataAdapter();
        private MySqlDataReader dr;


        public MySqlDataReader Dr
        {
            get { return dr; }
            set { dr = value; }
        }

        public MySqlDataAdapter Adap
        {
            get { return adap; }
            set { adap = value; }
        }

        public MySqlCommand Cmd
        {
            get { return cmd; }
            set { cmd = value; }
        }

        public MySqlConnection Conn
        {
            get { return conn; }
            set { conn = value; }
        }

        public DAL(IConfiguration configuration)
        {
            this.conn = new MySqlConnection(configuration.GetConnectionString("DefaultConnection"));
        }


        public DAL(string constr)
        {

            this.conn = new MySqlConnection(constr);

        }

   }

Some professionals here say that my DAL Class is terrible because of this

To be honest I didn't understand it very well. But no problem. You say it's not right to wrap Ado.net. So I don't do it anymore. There is just one problem. What happens if I have to change my db from MySql to MSSQL? How can I write code the way I can change my DB with changing just one file and not more. How should I do that?

Vahid2023
  • 863
  • 3
  • 11
  • 33
  • 1
    If you don't understand it, I suggest you read it again carefully, it's very important. If you have any questions about that answer feel free to make a post about it. – Charlieface Feb 28 '21 at 14:12
  • 1
    In addition I'd argue names like "Dr", "Adap" and "Cmd" make it even "worse". A few extra characters won't kill you and will make things much easier to read for others (and your future self) – pinkfloydx33 Feb 28 '21 at 14:58
  • 1
    If you don't want to go full ORM (EF), why not use Dapper? – insane_developer Feb 28 '21 at 15:22

1 Answers1

3

You should use interfaces. By example instead of returning "MySqlDbConnection", you could return a "IDbConnection".

In that case the caller will only know it's a database connection. No matter if it's a MySQL or SQL Server connection.

That way the only file that knows about MySql specifically will be your DAL class.

Those interfaces already exist (IDbConnection, IDbCommand, ...) so don't need to create it yourself.

Edit :

In the following example I modified a few things :

  • Use interfaces (or abstract classes)
  • Using naming guidelines + readonly keyword
  • Use full name for naming things (as suggested by @pinkfloydx33 in a comment) it will help readibility. For example the name of the parameter you used for the connection string was "constr" wich, for me, was meaning "constructor" at first (without reading the following line).

What is not fixed:

  • The usage of the DbConnection (as you mentionned in your post, you should read again the question you linked because if it's a production code it can/will cause so issues).

Possible improvements:

  • Using an ORM or a library like Dapper (as suggested by @insane_developer in a comment)

     public class DAL
     {
         private readonly IDbConnection _connection;
         private DbCommand _command = new MySqlCommand();
         private IDbDataAdapter = new MySqlDataAdapter();
         private IDataReader _dataReader;
    
         public IDataReader DataReader
         {
             get { return _dataReader; }
             set { _dataReader = value; }
         }
    
         public IDbDataAdapter Adapter
         {
             get { return _adapter; }
             set { _adapter = value; }
         }
    
         public DbCommand Cmd
         {
             get { return _command; }
             set { _command = value; }
         }
    
         public IDbConnection Connection
         {
             get { return _connection; }
             set { _connection = value; }
         }
    
         public DAL(IConfiguration configuration)
         {
             this._connection = new MySqlConnection(configuration.GetConnectionString("DefaultConnection"));
         }
    
         public DAL(string connectionString)
         {
             this._connection = new MySqlConnection(connectionString);
         }
    
    }
    

That way only this class knows about the DBMS type you're using.

Arcord
  • 1,724
  • 1
  • 11
  • 16