Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I will try to be as concise as I can:

The goal: trying to universalize a specific section of a project, that is dealing with the SQL database transactions.

To assist you with your answer, I've pasted the following (just for reference):

  • a usage code: GetTestOfTablTime() returns a DataTable

  • class: SQLDBInteraction is another class - responsible for the final(SQL transaction) stage

In this code below, I am constructing what I call: "Stored Procedure's Meta Data". That class is the one that holds all of the SQL Db SPs.

HTSPs (HT is the company's aliases) is holding each SP required) parameters. HTSPs class contains another sub Class. For all SPs Names, it only has const strings for each SP name.

public sealed class HTSPs
{

//so for example this is one of the members of this class - a stored procedure
//its mission: get evnents with specified id OF specified userId in spec' month, year..

    public sealed class GetTimesWithCustomerNames
    {
        //if I DO need Constructor for its parameters how do I properly format the constructor?
        public GetTimesWithCustomerNames()
        {
            Userid.ParameterName = ParNameUserid;
            Month.ParameterName = ParNameMonth;
            Year.ParameterName = ParNameYear;
            EventId.ParameterName = ParNameReasonid;

        }

        const string ParNameUserId = "@userId",
                     ParNameMonth = "@month",
                     ParNameYear = "@year",
                     ParNameEventId = "@eventId";

        public static SqlParameter Userid = new SqlParameter();
        public static SqlParameter Month = new SqlParameter();
        public static SqlParameter Year = new SqlParameter();
        public static SqlParameter EventId = new SqlParameter();            
    }
}

The issue is: how do I initialize the contractor? What is the proper way to have your simple customized StoredProcedure "MetaData"? I've currently completed the implementation of the method below (apart from that issue).

USAGE

This is a method that returns DataTable while using the HTSPs class / constructor.

using SPTime = HT_DBSchema.HTSPs.GetTimesWithCustomerNames;

private DataTable GetTestOfTablTime()
{
    SQLDBInteraction.DataContainer DC_Time = new SQLDBInteraction.DataContainer();

    SQLDBInteraction.SqlParamList parmsTime = new SQLDBInteraction.SqlParamList();
    Dictionary<SqlParameter, int> SqlCmdParDict = new Dictionary<SqlParameter, int>();
    parmsTime.SqlCmd.CommandType = CommandType.StoredProcedure;
    parmsTime.SqlCmd.CommandText = AppDb.MetaSqlSProc.Time.Name;
    parmsTime.SP_Name = AppDb.MetaSqlSProc.Time.Name;
    parmsTime.TableName = AppDb.MetaSqlTable.Time.Name;

    //While folowing implementation Does Work I comented it out to try using the SP Struct
    //ParmsTTime.SP_Params.Add(new SqlParameter(SPTime.ParNameMonth, 9));
    //ParmsTTime.SP_Params.Add(new SqlParameter(SPTime.ParNameReasonid, 1));
    //ParmsTTime.SP_Params.Add(new SqlParameter(SPTime.ParNameYear, 2012));
    //ParmsTTime.SP_Params.Add(new SqlParameter(SPTime.ParNameUserid, 3571));


    //here's where I'm currently stuck, in section below. trying to assign values for the SqlCommand
    parmsTime.SqlCmd.Parameters.AddWithValue(SPTime.ParNameMonth, 9);
    parmsTime.SqlCmd.Parameters.AddWithValue(SPTime.ParNameYear, 2012);
    parmsTime.SqlCmd.Parameters.AddWithValue(SPTime.ParNameReasonid, 1);
    SPTime.Userid.Direction = ParameterDirection.Input;
    SPTime.Userid.SqlValue = 3571;
    return DC_Time.LocalTbl_V3(ParmsTime);
 }

UPDATE

The last lines of the code above is trying to implement the parameters assignment, thus it will no longer be required to use:

SQLDBInteraction.SqlParamList.SP_Params (which is List<SqlParameter>).

And instead, I would really like to be able to use SQLDBInteraction.SqlParamList.SqlCmd.Parameters as it is already used for most of the required steps to interact with the database.

This is how I will drop some unnecessary usage of extra variables while at the same time I wanted to assign SqlCmd (parmsTime.SqlCmd.Parameters.Add(......)) with the struct - SPTime Real SqlParameters instead of using the strings that represents their name (e.g. parameter.name - (SPTime.ParNameMonth, someValue)).

Final stage- SQL transaction

The SQLDBInteraction class that does the transaction:

public class SQLDBInteraction
{
    public class SqlParamList
    {
        public SqlCommand SqlCmd = new SqlCommand();
        public List<SqlParameter> SP_Params = new List<SqlParameter>();
        public String SP_Name;
        public string TableName;
        public string SelectCommand;
        ///public SqlCommandType SelectedCmdType;
    }

    public class DataContainer
    {
        public DataTable LocalTbl_V3(SqlParamList Params)
        {
            SqlConnection sqlConnection;
            DataTable Usrs = new DataTable(Params.TableName);
            SqlDataAdapter sqlDataAdapter;

            using (sqlConnection = new SqlConnection(WebConfigurationManager.ConnectionStrings["HTConn"].ConnectionString))
            {
                sqlConnection.Open();

                using (Params.SqlCmd.Connection = sqlConnection)
                {
                    using (sqlDataAdapter = new SqlDataAdapter(Params.SqlCmd.CommandText, sqlConnection))
                    {
                        if (sqlDataAdapter.SelectCommand.Parameters.Count > 0 == false)
                        {
                            sqlDataAdapter.SelectCommand = Params.SqlCmd;
                            sqlDataAdapter.Fill(Usrs);
                        }
                    }
                }
            }
            return Usrs;
        }

I will really appreciate it if someone will find what am I doing wrong with the part of the stored procedure's parameters assigned to the SQL command.

share|improve this question
    
Have you read about ORMs, e.g. Entity Framework? Is there a strong reason to use DataTables rather than typed entities? Is there a strong reason to use stored procedures? Your code is absolutely not thread-safe and has too much of unnecessary classes involved in the simplest operations like reading the data from DB. –  almaz Dec 9 '12 at 18:14
    
@almaz say, do you want to see how exactly the SP LOOKS LIKE or Did You Really Know As To What exactly Does It Do In DB ? did you think it is something like : select * From Table ??, it has half a page or even almost a full page of code that makes a report of accouts –  LoneXcoder Dec 9 '12 at 18:19
    
@LoneXcoder almaz doesn't know what your stored procedure looks like, which is why I think he was asking you questions, not really giving suggestions. And ORMs can work with stored procedures. –  svick Dec 9 '12 at 18:32
2  
@LoneXcoder Well, using ORM could actually make your code simpler, it means you don't have to deal with SqlConnections ` and SqlDataAdapters and DataTables, you get a nice, strongly-typed collection of objects. –  svick Dec 9 '12 at 18:47
1  
@LoneXcoder can you please avoid syntax errors in your questions? It goes a long way to improve the quality of the question, which will bring you more and better answers. (Example of a tool for it: support.mozilla.org/en-US/kb/how-do-i-use-firefox-spell-checker ) –  ANeves Dec 10 '12 at 18:18

3 Answers 3

Maybe you are trying to do something like this

public interface DbProc
{
    public SqlCommand GetCommand();
}

public class MyProc : DbProc
{
    private SqlCommand _cmd;

    public const string COMMAND_TEXT = "spc_MyProc";
    public const string PARAM1 = "@Param1";
    public const string PARAM2 = "@Param2";        

    public MyProc(int param1, string param2)
    {
        _cmd = new SqlCommand(COMMAND_TEXT);
        _cmd.CommandType = CommandType.StoredProcedure;
        _cmd.Parameters.AddWithValue(PARAM1, param1);
        _cmd.Parameters.AddWithValue(PARAM2, param2);
    }

    public SqlCommand GetCommand()
    {
        return _cmd;
    }
}

public class Db
{
    public DataTable ExecuteProc(DbProc proc)
    {
        SqlCommand cmd = proc.GetCommand();
        ConfigureCommand(cmd);
        // execute the command into a data table
        return result;
    }

    // Set the common settings for all commands
    private void ConfigureCommand(SqlCommand cmd)
    {
        cmd.CommandTimeout = 10000;            
    }
}
share|improve this answer
    
hi andy , thanks for your answer, I am in a little time-out so I can't really check on that solution , but I will review this further next week hopefully , in first glance it does look as a fine approach , thanks I will look in to it and try implement it instead of my answer , so it will be the right answer eventually if it works . thanks a lot mate. –  LoneXcoder Jan 11 '13 at 18:34

Just to give you idea how your code would look like if you use Entity Framework:

using (var db = new HpDatabaseContext())
{
    return db.GetTimesWithCustomerNames(userId, month, year, reasonId).ToArray();
}
share|improve this answer
    
did you mention a 4 files scaterd around the folder xmls and some other UFO like extansions?, the key idea is to complete my code, then to compare it to EF (i am thinking of performance too i will check on this) ( by the way my implementaion logic is in these moments with this acutal question and goals to implemet querys as short as it could be you just need to initialize the struct : which table which sp you want to invoke.vwalla. just connect form fields to parameters in code behind=report is in page faster than ablink of an eye im ready to compare it to any new approach exists(speed wise) –  LoneXcoder Dec 9 '12 at 20:02
2  
You should not worry about performance since it is just a premature optimisation. And quite likely it will work faster than populating a DataTable with SqlDataAdapter. It's good to understand how things work under the hood, but if you want to learn how to write good maintainable production code it's better to start using proven techniques. The sooner you start using them the faster you'll learn how to use them properly. –  almaz Dec 9 '12 at 20:40

this is aventually what i am using (atleast For now)

...as there will ,i guess.. always be, further developments.

i will mark this as the answer soon , though you are still very welcome to post comments, or better yet, your improved version .

anyways this is an example for how a stored procedure is stored in my "DbSchema" namespace

    public static class RobSelect_Update_tblTC_CPA_ByDDL_paramNames
    {
        static SqlParameter aDepartment, bUid, cMonth, dYear, eUpdateFirst;

        public static List<SqlParameter> SqlParlstCPA(string SelectedDepartmentID, string SelectedUserID, string SelectedMonth, string selectedYear, string AbitValuIfUpdateFirst)
        {
            aDepartment = new SqlParameter( a_UsrDepartment, SelectedDepartmentID);
            bUid = new SqlParameter(b_UsrID, SelectedUserID);
            cMonth = new SqlParameter(c_Month, SelectedMonth);
            dYear = new SqlParameter(d_Year, selectedYear);
            eUpdateFirst = new SqlParameter(e_Updateit_bit, AbitValuIfUpdateFirst);


            List<SqlParameter> RetSqlParLst  = new List<SqlParameter>();
            RetSqlParLst.Add(aDepartment);
            RetSqlParLst.Add(bUid);
            RetSqlParLst.Add(cMonth);
            RetSqlParLst.Add(dYear);
            RetSqlParLst.Add(eUpdateFirst);

            return RetSqlParLst;
        }
        static readonly string a_UsrDepartment = "@Department",
                     b_UsrID = "@Uid",
                     c_Month = "@month",
                     d_Year = "@year",
                     e_Updateit_bit = "@updateItFirst";
    }
share|improve this answer
    
downvoters i dont count in those votes without a comment of why ,as it is being offensive approach Comments may help to explain what you think about it instead –  LoneXcoder Dec 17 '12 at 22:06

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.