Inserting an entire form into MySQL database

Question

Please review this.

public int InsertQuery(string Query)
{
    int LastInsertedID = -1;
    MySqlCommand MySqlCmd = new MySqlCommand(Query, this.sqlConn);
    try
    {
        MySqlCmd.ExecuteNonQuery();
        LastInsertedID = int.Parse(MySqlCmd.LastInsertedId.ToString());
    }
    catch (Exception excp)
    {
        Exception myExcp = new Exception("Could not execute INSERT Query.<br />" + Query + "<br /> Error: <br />" + excp.Message, excp);
        throw (myExcp);
    }
    return LastInsertedID;
}

public int AddChannel(NameValueCollection FormValues)
{
    string Keys = string.Join(",",FormValues.AllKeys);

    string Values = string.Join(",", FormValues.AllKeys.Select(key => String.Format("\"{0}\"", HttpContext.Current.Server.HtmlEncode(FormValues[key]))));
    return InsertQuery("INSERT INTO channels (" + Keys + ") VALUES (" + Values + ");");
}

Answer 1

Don't take shortcuts. Build a proper insert statement using a parameterized query and validate each of the form inputs that they're also of the proper type (ie., not sending a string into an integer field).

Do this to protect the integrity of your data, both from improper types and from malicious (or even unintended) attacks.

See this and other related questions on SO for constructing proper parameterized queries with MySql.

More than that, you don't want your class that deals with data inserts to also interact with your UI. These concerns should be seperated. Seperate your UI code from your business logic code from your data access code. Don't let these things mingle, because your UI could change, your logic could change, and your data strategy could change, and they could change independently.

Answer 2

As Anthony suggests, build a proper data access layer.
Never build SQL by string concatenation.

If you're sure you don't want the heavinesss usually associated with the use of an OR mapper such as Entity Framework or NHibernate, take a look at Dapper micro ORM. It is just one file to be added to project, is blazingly fast and simple. See here on how to insert the data.

By using a class instead of NameValueCollection, you'll also make your queries safer.