ARCHITETURAL QUESTION

Collapse
This topic is closed.
X
X
 
  • Time
  • Show
Clear All
new posts
  • Bruno Rodrigues

    ARCHITETURAL QUESTION

    Hi all

    I'm using the following code standart in class
    libraries. I will not split this function in more layers,
    but I want to know if there are some architetural
    improvement to do in my code. E.g: Database connection;
    Try/Catch use; DataTable as a set of rows; etc...
    Any suggestions are welcome.

    Thanks!
    Bruno Rodrigues

    =============== =============== =============== =============

    using System;
    using System.Data;
    using System.Data.Sql Client;

    namespace CodeDesign
    {
    /// <summary>
    /// Test class for code design.
    /// </summary>
    public class Test
    {
    SqlConnection conn = new SqlConnection
    ("Persist Security Info=False;Inte grated
    Security=False; database=Test;s erver=Computer; Connect
    Timeout=30;User ID=sa;Password= sa;");

    private void openDataBase()
    {
    try
    {
    conn.Open();
    }
    catch(Exception e)
    {
    throw new Exception("An
    error ocurred while opening the database.\n" + e.Message,
    e);
    }
    }

    private void closeDataBase()
    {
    try
    {
    conn.Close();
    }
    catch(Exception e)
    {
    throw new Exception("An
    error ocurred while closing the database.\n" + e.Message,
    e);
    }
    }

    /// <summary>
    /// Retrieves a specific record.
    /// </summary>
    /// <param name="id">Recor d ID.</param>
    /// <returns>DataRo w with record's
    data.</returns>
    public DataRow Return(int id)
    {
    openDataBase();

    string sql = "SELECT * FROM Tests
    WHERE PK = " + id.ToString();
    SqlDataAdapter adapter = new
    SqlDataAdapter( sql, conn);
    DataTable table = new DataTable();

    try
    {
    adapter.Fill(ta ble);
    return table.Rows[0];
    }
    catch(Exception e)
    {
    throw new Exception("An
    error ocurred while retrieving the records.\n" +
    e.Message, e);
    }
    finally
    {
    closeDataBase() ;
    }
    }


    /// <summary>
    /// Retrieves all records.
    /// </summary>
    /// <returns>DataTa ble with all
    records.</returns>
    public DataTable ReturnAll()
    {
    string sql = "SELECT * FROM
    Tests";
    SqlDataAdapter adapter = new
    SqlDataAdapter( sql, conn);
    DataTable table = new DataTable();

    try
    {
    adapter.Fill(ta ble);
    return table;
    }
    catch(Exception e)
    {
    throw new Exception("An
    error ocurred while retrieving all records.\n" +
    e.Message, e);
    }
    finally
    {
    closeDataBase() ;
    }
    }

    /// <summary>
    /// Returns the number of records.
    /// </summary>
    /// <returns>Numb er of records.</returns>
    public int Count()
    {
    string sql = "SELECT Count(PK) AS
    Total FROM Tests";
    SqlCommand command = new
    SqlCommand(sql, conn);

    try
    {
    return Convert.ToInt32
    (command.Execut eScalar());
    }
    catch(Exception e)
    {
    throw new Exception("An
    error ocurred while counting records.\n" + e.Message, e);
    }
    finally
    {
    closeDataBase() ;
    }
    }

    /// <summary>
    /// Delete a record.
    /// </summary>
    /// <param name="id">Recor d ID.</param>
    public void Delete(int id)
    {
    string sql = "DELETE FROM Tests
    WHERE PK = " + id.ToString();
    SqlCommand command = new
    SqlCommand(sql, conn);

    try
    {
    command.Execute NonQuery();
    }
    catch(Exception e)
    {
    throw new Exception("An
    error ocurred while deleting a record.\n" + e.Message, e);
    }
    finally
    {
    closeDataBase() ;
    }
    }
    }
    }
  • Manish Agarwal

    #2
    Re: ARCHITETURAL QUESTION

    see http://www.microsoft.com/resources/p...mpletelist.asp

    --
    -------------------------
    "Manish Agarwal"- <manishkrishan@ hotmail.com>

    "Bruno Rodrigues" <anonymous@disc ussions.microso ft.com> wrote in message
    news:070101c39d be$b19ded10$a30 1280a@phx.gbl.. .[color=blue]
    > Hi all
    >
    > I'm using the following code standart in class
    > libraries. I will not split this function in more layers,
    > but I want to know if there are some architetural
    > improvement to do in my code. E.g: Database connection;
    > Try/Catch use; DataTable as a set of rows; etc...
    > Any suggestions are welcome.
    >
    > Thanks!
    > Bruno Rodrigues
    >
    > =============== =============== =============== =============
    >
    > using System;
    > using System.Data;
    > using System.Data.Sql Client;
    >
    > namespace CodeDesign
    > {
    > /// <summary>
    > /// Test class for code design.
    > /// </summary>
    > public class Test
    > {
    > SqlConnection conn = new SqlConnection
    > ("Persist Security Info=False;Inte grated
    > Security=False; database=Test;s erver=Computer; Connect
    > Timeout=30;User ID=sa;Password= sa;");
    >
    > private void openDataBase()
    > {
    > try
    > {
    > conn.Open();
    > }
    > catch(Exception e)
    > {
    > throw new Exception("An
    > error ocurred while opening the database.\n" + e.Message,
    > e);
    > }
    > }
    >
    > private void closeDataBase()
    > {
    > try
    > {
    > conn.Close();
    > }
    > catch(Exception e)
    > {
    > throw new Exception("An
    > error ocurred while closing the database.\n" + e.Message,
    > e);
    > }
    > }
    >
    > /// <summary>
    > /// Retrieves a specific record.
    > /// </summary>
    > /// <param name="id">Recor d ID.</param>
    > /// <returns>DataRo w with record's
    > data.</returns>
    > public DataRow Return(int id)
    > {
    > openDataBase();
    >
    > string sql = "SELECT * FROM Tests
    > WHERE PK = " + id.ToString();
    > SqlDataAdapter adapter = new
    > SqlDataAdapter( sql, conn);
    > DataTable table = new DataTable();
    >
    > try
    > {
    > adapter.Fill(ta ble);
    > return table.Rows[0];
    > }
    > catch(Exception e)
    > {
    > throw new Exception("An
    > error ocurred while retrieving the records.\n" +
    > e.Message, e);
    > }
    > finally
    > {
    > closeDataBase() ;
    > }
    > }
    >
    >
    > /// <summary>
    > /// Retrieves all records.
    > /// </summary>
    > /// <returns>DataTa ble with all
    > records.</returns>
    > public DataTable ReturnAll()
    > {
    > string sql = "SELECT * FROM
    > Tests";
    > SqlDataAdapter adapter = new
    > SqlDataAdapter( sql, conn);
    > DataTable table = new DataTable();
    >
    > try
    > {
    > adapter.Fill(ta ble);
    > return table;
    > }
    > catch(Exception e)
    > {
    > throw new Exception("An
    > error ocurred while retrieving all records.\n" +
    > e.Message, e);
    > }
    > finally
    > {
    > closeDataBase() ;
    > }
    > }
    >
    > /// <summary>
    > /// Returns the number of records.
    > /// </summary>
    > /// <returns>Numb er of records.</returns>
    > public int Count()
    > {
    > string sql = "SELECT Count(PK) AS
    > Total FROM Tests";
    > SqlCommand command = new
    > SqlCommand(sql, conn);
    >
    > try
    > {
    > return Convert.ToInt32
    > (command.Execut eScalar());
    > }
    > catch(Exception e)
    > {
    > throw new Exception("An
    > error ocurred while counting records.\n" + e.Message, e);
    > }
    > finally
    > {
    > closeDataBase() ;
    > }
    > }
    >
    > /// <summary>
    > /// Delete a record.
    > /// </summary>
    > /// <param name="id">Recor d ID.</param>
    > public void Delete(int id)
    > {
    > string sql = "DELETE FROM Tests
    > WHERE PK = " + id.ToString();
    > SqlCommand command = new
    > SqlCommand(sql, conn);
    >
    > try
    > {
    > command.Execute NonQuery();
    > }
    > catch(Exception e)
    > {
    > throw new Exception("An
    > error ocurred while deleting a record.\n" + e.Message, e);
    > }
    > finally
    > {
    > closeDataBase() ;
    > }
    > }
    > }
    > }[/color]


    Comment

    • Lawrence Thurman

      #3
      Re: ARCHITETURAL QUESTION

      Well One thing is to Make all of you SQL strings constants. -
      with good names. -
      In fact I place any strings into Constants in all programs. - or a
      database.- but the SQL should be a constant


      "Bruno Rodrigues" <anonymous@disc ussions.microso ft.com> wrote in message
      news:070101c39d be$b19ded10$a30 1280a@phx.gbl.. .[color=blue]
      > Hi all
      >
      > I'm using the following code standart in class
      > libraries. I will not split this function in more layers,
      > but I want to know if there are some architetural
      > improvement to do in my code. E.g: Database connection;
      > Try/Catch use; DataTable as a set of rows; etc...
      > Any suggestions are welcome.
      >
      > Thanks!
      > Bruno Rodrigues
      >
      > =============== =============== =============== =============[/color]


      Comment

      • Nicholas Paldino [.NET/C# MVP]

        #4
        Re: ARCHITETURAL QUESTION

        Bruno,

        There are a number of issues here:

        - Your class does not implement IDisposable. Because you hold other
        resources on the class level which implement IDisposable, your class should
        do the same.

        - Your swallowing of the original exception and then re-throwing of a new
        exception is a very, VERY bad idea IMO. I think that you should let the
        original exception propogate. It's good that you are setting the inner
        exception, but using a general Exception is very bad. You should create a
        new Exception class specific to the error you are getting. So if you have
        an error opening the database, you should throw an OpenDatabaseExc eption, or
        something of the sort. Eric Gunnerson wrote a good article about using
        type-specific exceptions vs a value on the exception itself.

        - You should be using stored prcoedures for all of your queries. If you
        can't use stored procedures, at the least, use parameterized queries (which
        you might or might not prepare).

        - Instead of using openDatabase/closeDatabase and handling that yourself,
        just pass the connection object to the command. It will open and close the
        connection for you. This also reduces a lot of the error code that you have
        (when the exception is thrown, you will know you have a problem opening the
        connection from the internal exception, and your exception will indicate
        where it happened).

        - Implement Count as a property, not as a method. If implementing as a
        method, then change the method name to GetCount, to indicate it is
        performing some operation.

        Hope this helps.

        --
        - Nicholas Paldino [.NET/C# MVP]
        - mvp@spam.guard. caspershouse.co m

        "Bruno Rodrigues" <anonymous@disc ussions.microso ft.com> wrote in message
        news:070101c39d be$b19ded10$a30 1280a@phx.gbl.. .[color=blue]
        > Hi all
        >
        > I'm using the following code standart in class
        > libraries. I will not split this function in more layers,
        > but I want to know if there are some architetural
        > improvement to do in my code. E.g: Database connection;
        > Try/Catch use; DataTable as a set of rows; etc...
        > Any suggestions are welcome.
        >
        > Thanks!
        > Bruno Rodrigues
        >
        > =============== =============== =============== =============
        >
        > using System;
        > using System.Data;
        > using System.Data.Sql Client;
        >
        > namespace CodeDesign
        > {
        > /// <summary>
        > /// Test class for code design.
        > /// </summary>
        > public class Test
        > {
        > SqlConnection conn = new SqlConnection
        > ("Persist Security Info=False;Inte grated
        > Security=False; database=Test;s erver=Computer; Connect
        > Timeout=30;User ID=sa;Password= sa;");
        >
        > private void openDataBase()
        > {
        > try
        > {
        > conn.Open();
        > }
        > catch(Exception e)
        > {
        > throw new Exception("An
        > error ocurred while opening the database.\n" + e.Message,
        > e);
        > }
        > }
        >
        > private void closeDataBase()
        > {
        > try
        > {
        > conn.Close();
        > }
        > catch(Exception e)
        > {
        > throw new Exception("An
        > error ocurred while closing the database.\n" + e.Message,
        > e);
        > }
        > }
        >
        > /// <summary>
        > /// Retrieves a specific record.
        > /// </summary>
        > /// <param name="id">Recor d ID.</param>
        > /// <returns>DataRo w with record's
        > data.</returns>
        > public DataRow Return(int id)
        > {
        > openDataBase();
        >
        > string sql = "SELECT * FROM Tests
        > WHERE PK = " + id.ToString();
        > SqlDataAdapter adapter = new
        > SqlDataAdapter( sql, conn);
        > DataTable table = new DataTable();
        >
        > try
        > {
        > adapter.Fill(ta ble);
        > return table.Rows[0];
        > }
        > catch(Exception e)
        > {
        > throw new Exception("An
        > error ocurred while retrieving the records.\n" +
        > e.Message, e);
        > }
        > finally
        > {
        > closeDataBase() ;
        > }
        > }
        >
        >
        > /// <summary>
        > /// Retrieves all records.
        > /// </summary>
        > /// <returns>DataTa ble with all
        > records.</returns>
        > public DataTable ReturnAll()
        > {
        > string sql = "SELECT * FROM
        > Tests";
        > SqlDataAdapter adapter = new
        > SqlDataAdapter( sql, conn);
        > DataTable table = new DataTable();
        >
        > try
        > {
        > adapter.Fill(ta ble);
        > return table;
        > }
        > catch(Exception e)
        > {
        > throw new Exception("An
        > error ocurred while retrieving all records.\n" +
        > e.Message, e);
        > }
        > finally
        > {
        > closeDataBase() ;
        > }
        > }
        >
        > /// <summary>
        > /// Returns the number of records.
        > /// </summary>
        > /// <returns>Numb er of records.</returns>
        > public int Count()
        > {
        > string sql = "SELECT Count(PK) AS
        > Total FROM Tests";
        > SqlCommand command = new
        > SqlCommand(sql, conn);
        >
        > try
        > {
        > return Convert.ToInt32
        > (command.Execut eScalar());
        > }
        > catch(Exception e)
        > {
        > throw new Exception("An
        > error ocurred while counting records.\n" + e.Message, e);
        > }
        > finally
        > {
        > closeDataBase() ;
        > }
        > }
        >
        > /// <summary>
        > /// Delete a record.
        > /// </summary>
        > /// <param name="id">Recor d ID.</param>
        > public void Delete(int id)
        > {
        > string sql = "DELETE FROM Tests
        > WHERE PK = " + id.ToString();
        > SqlCommand command = new
        > SqlCommand(sql, conn);
        >
        > try
        > {
        > command.Execute NonQuery();
        > }
        > catch(Exception e)
        > {
        > throw new Exception("An
        > error ocurred while deleting a record.\n" + e.Message, e);
        > }
        > finally
        > {
        > closeDataBase() ;
        > }
        > }
        > }
        > }[/color]


        Comment

        Working...