aliazik
aliazik

Reputation: 195

C# - Can't insert data into MySQL database

I am making an application where I scrape a certain website. The website has a bunch of players, and I scrape each one of their profile pages. Their profile pages contains information like Name, Level, World and Last Login date.

So I made an object called Player. And then I add all their data to a list.

public static List<Player> Players = new List<Player> { };

public class Player
    {
        public string Name { get; set; }
        public string Sex { get; set; }
        public string Vocation { get; set; }
        public int Level { get; set; }
        public int Achievements { get; set; }
        public string World { get; set; }
        public string LastLogin { get; set; }
        public string AccountStatus { get; set; }
        public int OnlineStatus { get; set; }
    }

I add the data like this:

new Player { Name = playerName, Sex = playerSex, Vocation = playerVocation, Level = playerLevel, Achievements = playerAchievements, World = playerWorld, LastLogin = playerLastLogin, AccountStatus = playerAccountStatus, OnlineStatus = playerOnlineStatus };

I now want to add all the players to my MySQL database, but I cannot seem to understand how to insert the data.

I established a connection and I tried to SELECT data and it works fine. So my connection is not bad. But inserting seems to not work.

Here's my code I add to insert the data into the tables:

string connString = "Server=localhost;Port=3306;Database=rookstat;Uid=XXXXXXX;password=XXXXXX;";
MySqlConnection conn = new MySqlConnection(connString);
MySqlCommand command = conn.CreateCommand();

foreach (Player rooker in Players)
{
command.CommandText = "Insert into rookstayers (id, name, sex, vocation, level, achievements, world, lastlogin, accountstatus, onlinestatus) values('', rooker.Name, rooker.Sex, rooker.Vocation, rooker.Level, rooker.Achievements, rooker.World, rooker.LastLogin, rooker.AccountStatus, rooker.OnlineStatus)";
conn.Open();
command.ExecuteNonQuery();
}

conn.Close();

What am I doing wrong? I am unsure about the values i insert. Is it supposed to be rooker.Name or what?

Upvotes: 2

Views: 2520

Answers (3)

cubski
cubski

Reputation: 3248

Try the following:

using(var connection = new MySqlConnection(connectionString)) 
{
    connection.Open();
    using(var command = new MySqlCommand()) 
    {
        command.Connection = connection;
        command.CommandText = @"INSERT INTO rookstayers (name, sex, vocation, level, achievements, world, lastlogin, accountstatus, onlinestatus) VALUES (@name, @sex, @vocation, @level, @achievements, @world, @lastLogin, @accountStatus, @onlineStatus)";
        command.Prepare();

        foreach(var rooker in Players) 
        {
            command.Parameters.Clear();
            command.Parameters.AddWithValue("@name", rooker.Name);
            command.Parameters.AddWithValue("@sex", rooker.Sex);
            command.Parameters.AddWithValue("@vocation", rooker.Vocation);
            command.Parameters.AddWithValue("@level", rooker.Level);
            command.Parameters.AddWithValue("@achievements", rooker.Achievements);
            command.Parameters.AddWithValue("@world", rooker.World);
            command.Parameters.AddWithValue("@lastLogin", rooker.LastLogin);
            command.Parameters.AddWithValue("@accountStatus", rooker.AccountStatus);
            command.Parameters.AddWithValue("@onlineStatus", rooker.OnlineStatus);
            command.ExecuteNonQuery();
        }
    }
}

Note the differences:

  1. Wrapping disposable objects (i.e. MySQLConnection and MySQLCommand) in using statements.
  2. Updated the SQL insert command to use SQL Parameters. In this case, "@name" will be replaced with the value of rooker.Name when the command is executed.
  3. Excluded id column in the insert statement as I assume that is the identity column so you don't have to explicitly include that.
  4. Lastly, use of command.Prepare(). As specified in the MySQL .NET Connector documentation here, it can can provide significant performance improvements on queries that are executed more than once.

Upvotes: 2

Steve
Steve

Reputation: 216243

The way in which you build your query creates a single string with your variables names treated as literal strings.

Instead you need a parameterized query like this

string connString = "Server=localhost;Port=3306;Database=rookstat;Uid=XXXXXXX;password=XXXXXX;";
using(MySqlConnection conn = new MySqlConnection(connString))
using(MySqlCommand command = conn.CreateCommand())
{
    conn.Open();
    command.CommandText = @"Insert into rookstayers 
             (id, name, sex,   vocation, level, achievements, 
              world, lastlogin, accountstatus, onlinestatus) 
       values('', @name, @sex, @vocation, @level, @Achievements,
              @World, @LastLogin, @AccountStatus, @OnlineStatus)";

    command.Parameters.Add("@name", MySqlDbType.VarChar);
    command.Parameters.Add("@sex", MySqlDbType.VarChar);
    command.Parameters.Add("@vocation", MySqlDbType.VarChar);
    command.Parameters.Add("@level", MySqlDbType.Int32);
    .... and so on for all the parameters placeholders

    command.Prepare();
    foreach (Player rooker in Players)
    {
          command.Parameters["@name"].Value = rooker.Name;
          command.Parameters["@sex"].Value = rooker.Sex;
          command.Parameters["@vocation"].Value = rooker.Vocation;
          command.Parameters["@level"].Value = rooker.Level;

          ... and so on for the other parameters defined
          command.ExecuteNonQuery();
    }
}

In this way, BEFORE entering the loop, you define your sql command and create the parameters in the MySqlCommand Parameters collection without setting the values. And, as suggested below by Mr adding the call to command.Prepare could enhance the performance.

Inside the loop, for each parameter defined, you could set its value and call the command to execute.

I wish also to point to the using statement that ensure the proper closing and disposing of the objects and the use of Add with proper parameter type set for each parameter instead of AddWithValue. AddWithValue is a shortcut with numerous drawbacks as you can read in this article Can we stop using AddWithValue already?

A last advice, if this kind of transformations between objects and data happens very often then you should invest a bit of your time to learn to use an ORM tool like Dapper

EDIT: Looking at your whole code, it seems that you miss an important step in your scraping. You need to add the Player to your list of Players

private async void insertTimer_Tick(object sender, EventArgs e)
{
     if (onlineList.Items.Count > 0)
     {
         for (int i=0; i<onlineList.Items.Count; i++)
         {
              CurrentPlayer = onlineList.Items[i].ToString();
              var playerProfile = await GetData();
              foreach (var row in playerProfile)
              {
                  .... gets player data ....
              }

              // Creates a new Player and add to the insertion list
              Players.Add(new Player 
              { 
                    Name = playerName, 
                    Sex = playerSex, 
                    Vocation = playerVocation, 
                    Level = playerLevel, 
                    Achievements = playerAchievements, 
                    World = playerWorld, 
                    LastLogin = playerLastLogin, 
                    AccountStatus = playerAccountStatus, 
                    OnlineStatus = playerOnlineStatus 
               };
          }

          // And now the inserts goes here
          .......

Upvotes: 6

Darren
Darren

Reputation: 70718

You are passing string literals to the query, not the variable values:

Try adjusting your query to:

command.CommandText = "Insert into rookstayers (id, name, sex, vocation) values('', @name, @sex, @vocation)";

Then Parameters.AddWithValue as shown below:

command.Parameters.AddWithValue("@name", rooker.Name);
command.Parameters.AddWithValue("@sex", rooker.Sex);
command.Parameters.AddWithValue("@vocation", rooker.Vocation);

etc

This will also help prevent SQL injection.

Upvotes: 4

Related Questions