0

I have a probleme : this the code :

private void modifier_Click(object sender, EventArgs e)
    {
        SqlConnection con = new SqlConnection(@"Data Source = (LocalDB)\MSSQLLocalDB; AttachDbFilename = C:\Users\ElliteBook\source\repos\Ecole Prive El Faouz\Data.mdf; Integrated Security = True");
        con.Open();

        string query = "update eleve set nom = '" +nom.Text+ "'where matricule = '" +recherche.Text+ "'";
        SqlDataAdapter sda = new SqlDataAdapter(query, con);
        sda.SelectCommand.ExecuteNonQuery();
        con.Close();
        MessageBox.Show("'"+recherche.Text+"'")
    }

and this the table sql :

CREATE TABLE [dbo].[eleve] (
[matricule]     INT            IDENTITY (10000, 1) NOT NULL,
[nom]           NVARCHAR (MAX) NULL,
[prenom]        NVARCHAR (MAX) NULL,
[nom_pere]      NVARCHAR (MAX) NULL,
[datenaissance] NVARCHAR (MAX) NULL,
[nationnalite]  NVARCHAR (MAX) NULL,
[telephone1]    NVARCHAR (MAX) NULL,
[telephone2]    NVARCHAR (MAX) NULL,
[classe]        NVARCHAR (MAX) NULL,
[remarque]      NVARCHAR (MAX) NULL,
[nom_mere]      NVARCHAR (MAX) NULL,
PRIMARY KEY CLUSTERED ([matricule] ASC)

the error is :

System.Data.SqlClient.SqlException : 'Conversion failed when converting the varchar value 'Rechercher' to data type int.'

enter image description here

mohamed-mhiri
  • 202
  • 3
  • 22
  • Don't inject your values! Parametrise your SQL! Fix the huge security flaw and you'll very likely fix the problem. – Thom A Feb 06 '20 at 10:55
  • matricule is int, you need to convert it to nvarchar – Red Devil Feb 06 '20 at 10:56
  • 2
    Also, why are *all* of your columns an `nvarchar(MAX)`? Do you *seriously* need over 1 billion characters to store someone's name or telephone number? Don't above the `MAX` data type; use an appropriate length. I doubt you need more than 20 characters for a phone number. You certainly do not be 1 Billion (1,000,000,000) characters. – Thom A Feb 06 '20 at 10:56
  • @RedDevil `IDENTITY` must be on a numerical data type, that only allows integer values (i.e. `int`, `decimal(7,0)`). A column of the datatype `nvarchar` **cannot** have the `IDENTITY` property as it isn't a numerical data type. – Thom A Feb 06 '20 at 10:57
  • @Larnu In the where clause we need to convert the column matricule to nvarchar. Because he is concatenating int with string – Red Devil Feb 06 '20 at 10:59
  • Remove the single quotes around `recherche.Text`, however you need to fix your sql injection issues – TheGeneral Feb 06 '20 at 11:03
  • The OP shouldn't be converting it, @RedDevil , the OP should be ***parametrising*** it. I cannot stress that enough. Then the error will never happen, as they will be passing an `int` parameter. – Thom A Feb 06 '20 at 11:03
  • Does this answer your question? [Why do we always prefer using parameters in SQL statements?](https://stackoverflow.com/q/7505808/2029983) (I have intentionally not gold badged, as mine are SQL related and the problem is C#). – Thom A Feb 06 '20 at 11:05
  • BTW you don't need `SqlDataAdapter` to execute a command. That class is only used to specify how data is loaded and saved from DataTables. – Panagiotis Kanavos Feb 06 '20 at 11:10

2 Answers2

3

If you want to update RDBMS Table, why use SqlDataAdapter? Let's just execute a command. Let's extract a method (i.e. separate business logic and UI):

  private bool UpdateEleve(string name, string key) {
    int matricule;

    // RDBMS want's int, mot string; let's ensure it
    if (!int.TryParse(key), out matricule) 
      return false;

    // wrap IDisposable into using 
    using (SqlConnection con = new SqlConnection(...)) {
      con.Open();

      // Keep queries readable and paramterized
      string query =
        @"update eleve 
             set nom = @prm_Nom
           where matricule = @prm_Matricule";

      using (var q = new SqlCommand(query, con)) {
        //TODO: q.Parameters.Add("@prm_Nom", RDMBS_TYPE).Value = ... is a better choice
        // I've put AddWithValue since I don't know RDMBS_TYPE
        q.Parameters.AddWithValue("@prm_Nom", name);
        q.Parameters.AddWithValue("@prm_Matricule", matricule);

        q.ExecuteNonQuery();

        return true;
      }
    } 
  }

Then call it:

  private void modifier_Click(object sender, EventArgs e) {
    if (UpdateEleve(nom.Text, recherche.Text))
      MessageBox.Show($"'{recherche.Text}'")
    else {
      if (recherche.CanFocus)
        recherche.Focus();

      MessageBox.Show($"{recherche.Text} is not a valid integer") 
    }
  }
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215
2

The commenters explained the problems with this code. There are many duplicate questions for each of those serious problems. Dmitry Bychenko shows how easy it is to get this right, in fact with a bit of formatting, the code can even use fewer lines than the question. The SqlCommand object can be created separately and reused too.

Another option is to use StackOverflow's Dapper and reduce the call to a couple of lines :

var matricule=int.Parse(recherce.Text);

var query="update eleve set nom = @nom where matricule = @matricule ";
using(var con = new SqlConnection(connString))
{
    con.Execute(query,new {nom=nom.Text,matricule});
}

Dapper's Execute creates the command and parameters, opens the connection and executes the command. The parameter names are taken from the anonymout type's field names, so nom becomes @nom etc.

In C#, out variables can be used to test and convert the input in the same line :

var query="update eleve set nom = @nom where matricule = @matricule ";

if(int.TryParse(recherce.Text,out var matricule))
{
    using(var con = new SqlConnection(connString))
    {
        con.Execute(query,new {nom=nom.Text,matricule});
    }
}
else
{
    //Warn about the bad input here
}
Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236