Search code examples
c#classabstract-class

Why is my stopwatch/timespan not retaining its information throughout methods?


I am currently working on a problem that involves abstract classes and stopwatches. I have two classes, SQL and Oracle. They both take a string for a connection code (this stuff doesn't actually do anything, but I am trying to make this a bit realistic). I want to start a stopwatch, then stop it in a different method - but the Timespan always says 00:00...

Am I accessing my parent class' properties correctly?

I have tried initializing my stopwatches and timespan in different places.

public class Program
    {
        public static void Main(string[] args)
        {
           // ConnectionManagement management = new ConnectionManagement();
           // management.SetUpOptions();


        }
    }

    public class ConnectionManagement
    {
        public void SetUpOptions()
        {
            while (true)
            {
                SqlConnection sqlGatherer = new SqlConnection("placeholder");
                OracleConnection oracleGatherer = new OracleConnection("placeholder");

                Console.WriteLine("1. Open an SQL connection.");
                Console.WriteLine("2. Close an SQL connection.");
                Console.WriteLine("3. Open an Oracle connection.");
                Console.WriteLine("4. Close an SQL connection.");
                string choice = Console.ReadLine();

                if (choice == "1")
                {
                    Console.WriteLine("Enter your connection string.");
                    string enteredConnectionString = Console.ReadLine();
                    sqlGatherer.ConnectionString = enteredConnectionString;
                    sqlGatherer.OpenConnection();
                }
                else if (choice == "2")
                {
                    sqlGatherer.CloseConnection();
                }
                else if (choice == "3")
                {
                    Console.WriteLine("Enter your connection string.");
                    string enteredConnectionString = Console.ReadLine();
                    oracleGatherer.ConnectionString = enteredConnectionString;
                    oracleGatherer.OpenConnection();
                }
                else if (choice == "4")
                {
                    oracleGatherer.CloseConnection();
                }
                else
                {
                    Console.WriteLine("That was not a valid option.");
                }
            }

        }
    }

    public abstract class DataBaseConnection
    {

        public string ConnectionString { get; set; }
        public TimeSpan Timeout { get; set; }
        public Stopwatch OracleStoppy { get; set; }
        public Stopwatch SqlStoppy { get; set; }
        public abstract void OpenConnection();
        public abstract void CloseConnection();

    }

    public class SqlConnection : DataBaseConnection
    {

        private bool CurrentConnection = false;

        public SqlConnection()
        {
            Timeout = new TimeSpan();
            SqlStoppy = new Stopwatch();
        }

        public SqlConnection(string connectionString)
        {

            Timeout = new TimeSpan();
            SqlStoppy = new Stopwatch();
            if (connectionString == null || String.IsNullOrWhiteSpace(connectionString))
            {
                throw new ArgumentException("Program has an invalid SQL connection string.");
            }
            else
            {
                this.ConnectionString = connectionString;
            }
        }

        public override void OpenConnection()
        {
            if (CurrentConnection == true)
            {
                throw new Exception("A connection has already been established.");
            }
            else
            {
                Console.WriteLine("SQL connection established.");
                SqlStoppy.Start();
                CurrentConnection = true;

            }

        }

        public override void CloseConnection()
        {
            if (CurrentConnection == false)
            {
                SqlStoppy.Stop();
                TimeSpan reportedTimeout = Timeout;
                Console.WriteLine("Connection closed. \nThe connection was active for {0}", reportedTimeout);
                SqlStoppy.Reset();
                CurrentConnection = false;

            }
            else
            {
                throw new Exception("There is no SQL connection to close.");
            }

        }
    }

    public class OracleConnection : DataBaseConnection
    {
        private bool CurrentConnection = false;

        public OracleConnection()
        {
            Timeout = new TimeSpan();
            OracleStoppy = new Stopwatch();
        }

        public OracleConnection(string connectionString)
        {
            Timeout = new TimeSpan();
            OracleStoppy = new Stopwatch();
            if (connectionString == null || String.IsNullOrWhiteSpace(connectionString))
            {
                throw new Exception("Program has an invalid Oracle connection string.");
            }
            else
            {
                this.ConnectionString = connectionString;
            }
        }
        public override void OpenConnection()
        {
            if (CurrentConnection == true)
            {
                throw new Exception("A connection has already been established.");
            }
            else
            {
                Console.WriteLine("Oracle connection established.");
                OracleStoppy.Start();
                CurrentConnection = true;
            }

        }

        public override void CloseConnection()
        {
            if (CurrentConnection == false)
            {
                throw new Exception("There is no Oracle connection to close.");
            }
            else
            {
                OracleStoppy.Stop();
                this.Timeout = OracleStoppy.Elapsed;
                Console.WriteLine("Connection closed. \nThe connection was active for {0}", Timeout);
                OracleStoppy.Reset();
                CurrentConnection = false;
            }

        }
    }

After I close an opened connection, the method should print how long it was opened for, or the stopwatch duration. The same problem happened with my CurrentConnection variable, so I put it as a private variable in each class. But isn't the point of having a parent class like this to have common properties that its children classes can interact with?


Solution

  • Problem

    I see that in oracle you have:

    this.Timeout = OracleStoppy.Elapsed;
    Console.WriteLine("Connection closed. \nThe connection was active for {0}", Timeout);
    

    But in sql:

    TimeSpan reportedTimeout = Timeout;
    Console.WriteLine("Connection closed. \nThe connection was active for {0}", reportedTimeout);
    

    This should be

    this.Timeout = SqlStoppy.Elapsed;
    Console.WriteLine("Connection closed. \nThe connection was active for {0}", Timeout);
    

    Isn't it?

    Advices

    You use a parent class, so you don't need two stopwatches, only one is needed else inheritence is useless and you repeat code.

    You should consider remove some setters too.

    And move CurrentConnection that is repeated, as protected.

    This should be:

    public abstract class DataBaseConnection
    {
        protected bool CurrentConnection;
    
        public string ConnectionString { get; }
        public TimeSpan Timeout { get; }
        public Stopwatch Stoppy { get; }
        public abstract void OpenConnection();
        public abstract void CloseConnection();
    
        public DataBaseConnection()
        {
          Timeout = new TimeSpan();
          Stoppy = new StopWatch();
        }
    }
    

    Also use call pattern between constructors:

    public OracleConnection()
    {
      ...
    }
    
    public OracleConnection(string connectionString)
      : this()
    {
      ...
    }
    

    You repeat too many same code and you really should refactor and better abstract things.

    When code is repeated you can:

    • create one method and call it instead of repeated code.

    • sometimes you can move that in a parent class.

    When variables is repeated in child classes you can remove them to have only one in a parent class.

    You do that playing with modifiers like public, private, protected, internal, abstract, virtual, override...

    You should also rename some vars to be more consistent and coherent.

    The rule is that a name must be simple and designate exactly what it is.

    For example Chrono may be better that Stoppy as well as index or indexRowis better that i.

    So here Duration or Elapsed may be better than Timeout that indicates the delay to stop a connection attempt.

    Also because classes are named Connection you don't need to specify OpenConnection and CloseConnection: Open and Close is sufficient.

    Perhaps you will find this usefull:

    How to choose between private and protected access modifier

    What is polymorphism