Oracle Database Communication/Operation

Question

I am creating an application that will move specific data from one Oracle database and moving it to a MySQL database. I created this class to help keep myself organized and this class holds all of the operations/communication that will happen with this database. I have learned C# on my own with no formal training I was just wanting some pointers that could help me make my code to be acceptable if I was working as a developer for a company.

class Oracle
{
    //private properties
    private OracleConnection oConnection;
    private XmlDocument oXML;
    private string oUID;
    private string oPassword;
    private string oSecurity;
    private string oDb;
    private string oConString;
    private OracleDataAdapter oCommand;
    private string oScript;
    private DataSet oData;
    private DataTable oTable;
    public Oracle()
    {
        oConnection = new OracleConnection();
        oXML = new XmlDocument();
        try
        {

            oXML.Load(@"C:\Users\V64016\documents\visual studio 2013\Projects\Test_DB_Connection\Test_DB_Connection\Resources\Test.xml");
            oSecurity = oXML.SelectSingleNode("/Connections/Oracle/Security").InnerText;
            oDb = oXML.SelectSingleNode("/Connections/Oracle/Database").InnerText;
            oUID = oXML.SelectSingleNode("/Connections/Oracle/UId").InnerText;
            oPassword = Encrypt.DecryptString(oXML.SelectSingleNode("/Connections/Oracle/Pwd").InnerText,oUID);
            oConString = "Data Source=" + oDb + ";User Id=" + oUID + ";Password=" + oPassword + ";";
        }
        catch(Exception ex)
        {
            Console.WriteLine(ex.ToString());
            Debug.Print(ex.ToString());
        }
    }
    //method that handles opening the connection to the  database
    public void Open()
    {
        try
        {
            oConnection.ConnectionString = oConString;
            oConnection.Open();
            Console.WriteLine("Oracle DB Connection Established");
        }
        catch(Exception ex)
        {
            Console.WriteLine(ex.ToString());
            Debug.Print(ex.ToString());
        }
    }
    //method that handles closing the connection to the database
    public void Close()
    {
        try
        {
            oConnection.Close();
            Console.WriteLine("Oracle DB Connection Closed");
        }
        catch(Exception ex)
        {
            Console.WriteLine(ex.ToString());
            Debug.Print(ex.ToString());
        }
    }
    //method that handles pulling the data from a table
    public DataTable Pull()
    {
        oCommand = new OracleDataAdapter();
        oData = new DataSet();
        oTable = new DataTable();
        try
        {
            oScript = System.IO.File.ReadAllText(@"C:\Users\V64016\documents\visual studio 2013\Projects\Test_DB_Connection\Test_DB_Connection\Resources\CASE_SELECT.sql");
            oCommand.SelectCommand = new OracleCommand(oScript, oConnection);
            oCommand.Fill(oData, "uds9");
            oTable = oData.Tables["uds9"];
        }
        catch(Exception ex)
        {
            Console.WriteLine(ex.ToString());
            Debug.Print(ex.ToString());
        }
        return oTable;
    }
}

Answer 1

The code looks good, however, some point I like to mention about readability..

I can see there are multiple try-catch blocks. Every catch block does same thing. You can easily terminate duplicate code.

public static void LogThisCode(Action action)
{
    try
    {
        action();
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.ToString());
        Debug.Print(ex.ToString());
    }
}

Call it like this:

public void Open()
{
    LogThisCode(() => {

        oConnection.ConnectionString = oConString;
        oConnection.Open();
        Console.WriteLine("Oracle DB Connection Established");

    });  
}

---

• You don't need all the variables to be global. Avoid using them. They aren't easy to track and create confusion when code grows.

• I've moved oConnection.ConnectionString = oConString; inside constructor to avoid global variable.

• Instead of comments on methods, use XML Documentation Comments

• Always check and remove any unused code

• You can use String.Format instead of concatenate strings

• I would suggest you to eliminate all hard coded strings by introducing a class having constants.

---

After all above changes code will look like this:

class Oracle
{
    private OracleConnection oConnection;
    private OracleDataAdapter oCommand;

    public static void LogThisCode(Action action)
    {
        try
        {
            action();
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.ToString());
            Debug.Print(ex.ToString());
        }
    }

    public Oracle()
    {
        LogThisCode(() =>
        {
            oConnection = new OracleConnection();
            var oXML = new XmlDocument();
            oXML.Load(@"C:\Users\V64016\documents\visual studio 2013\Projects\Test_DB_Connection\Test_DB_Connection\Resources\Test.xml");

            string oDb = oXML.SelectSingleNode("/Connections/Oracle/Database").InnerText;
            string oUID = oXML.SelectSingleNode("/Connections/Oracle/UId").InnerText;
            string oPassword = Encrypt.DecryptString(oXML.SelectSingleNode("/Connections/Oracle/Pwd").InnerText, oUID);
            string oConString = string.Format("Data Source={0};User Id={1};Password={2};", oDb, oUID, oPassword);
            oConnection.ConnectionString = oConString;
        });
    }

    /// <summary>
    /// Method that handles opening the connection to the  database
    /// </summary>
    public void Open()
    {
        LogThisCode(() =>
        {
            oConnection.Open();
            Console.WriteLine("Oracle DB Connection Established");  
        });
    }

    /// <summary>
    /// Method that handles closing the connection to the database
    /// </summary>
    public void Close()
    {
        LogThisCode(() =>
        {
            oConnection.Close();
            Console.WriteLine("Oracle DB Connection Closed");
        });
    }

    /// <summary>
    /// Method that handles pulling the data from a table
    /// </summary>
    /// <returns></returns>
    public DataTable Pull()
    {
        DataTable oTable = new DataTable();

        LogThisCode(() =>
        {
            string oScript = System.IO.File.ReadAllText(@"C:\Users\V64016\documents\visual studio 2013\Projects\Test_DB_Connection\Test_DB_Connection\Resources\CASE_SELECT.sql");
            oCommand.SelectCommand = new OracleCommand(oScript, oConnection);

            DataSet oData = new DataSet();
            oCommand.Fill(oData, "uds9");
            oTable = oData.Tables["uds9"];
        });

        return oTable;
    }
}