1

I am currently writing a method that is supposed to SELECT * FROM users so I can get the userid, then use that userid and use it in a comment with a commenttext. This will be inserted into a own db table named Comment. The code is messy, and I am wondering if there is anyways to get the userid with session, which is more simple. Here I had to write a hell lot of code.

 public void CreateComment(TextBox a, TextBox b)
        {
            HttpContext context = HttpContext.Current;
            string username = context.Session["username"].ToString();
            string password = context.Session["password"].ToString();

        SqlConnection conn = new SqlConnection(ConfigurationManager.ConnectionStrings["ForumDatabaseConnectionString"].ConnectionString);
        SqlCommand cmd1 = new SqlCommand("Select * From Users Where username=@username and password=@password", conn);
        cmd1.Parameters.AddWithValue("@username", username);
        cmd1.Parameters.AddWithValue("@password", password);
        SqlDataAdapter sda = new SqlDataAdapter(cmd1);
        DataTable dt = new DataTable();
        sda.Fill(dt);
        conn.Open();

        string commentId = a.Text; // Kan droppe id om vi skal ha autoincrement.
        string commentText = b.Text;
        string userId = dt.Rows[0][0].ToString();
        string time = DateTime.Now.ToShortDateString();

        // MÅ FÅ KOMMENTARENE TIL Å HØRE TIL EN TOPIC!
        SqlConnection con = new SqlConnection(ConfigurationManager.ConnectionStrings["ForumDatabaseConnectionString"].ConnectionString);
        con.Open();
        string sql = "INSERT INTO Comment(commentid, commenttext, userid, time) VALUES(@param1,@param2, @param3, @param4)";
        SqlCommand cmd = new SqlCommand(sql, con);
        cmd.Parameters.AddWithValue("@param1", commentId);
        cmd.Parameters.AddWithValue("@param2", commentText);
        cmd.Parameters.AddWithValue("@param3", userId);
        cmd.Parameters.AddWithValue("@param4", time);


        cmd.CommandType = CommandType.Text;
        cmd.ExecuteNonQuery();
    }
Adrian
  • 57
  • 6
  • Some unrelated tips: SqlConnection, SqlCommand and SqlDataAdapter are all IDisposable so each should be in a `using` block. `SELECT *` is a [bad idea](https://stackoverflow.com/questions/321299/what-is-the-reason-not-to-use-select). This code will crash in the scenario where the first query doesn't find a match. I'm really hoping those passwords are not in plain text. Prefer ExecuteScalar rather than the Data Adapter... You only need the first value from the first row. – Richardissimo Feb 11 '19 at 22:29
  • Consider using better names than `@param1,2,3,4`. The `time` variable is badly named - it is holding a date. Pass that date as a dotnet `DateTime` type rather than a string, and pass it as an `SqlDbType.Date` parameter after reading [can we stop using AddWithValue](https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/). Consider using `DateTime.Today` rather than `DateTime.Now` to get the date. – Richardissimo Feb 11 '19 at 22:32

0 Answers0