-2

I refer to the checked answer "How to get last inserted id?"

So far here is what I've done.

 SqlCommand cmdInsert = new SqlCommand("INSERT INTO normal_asset(n_asset_serialNo,n_asset_propertyNo,n_asset_brandModel,n_asset_unitCost,n_asset_acquisitionDate,equitype_id,region_id) OUTPUT INSERTED.N_Asset_ID VALUES ('" + assetSerial + "','" + assetProperty + "','" + assetBrand + "','" + assetCost + "','" + assetAcquisition + "','" + assetEquipment + "','" + Session["usersRegion"].ToString() + "')");

 cmdInsert.CommandType = System.Data.CommandType.Text;
 cmdInsert.Connection = conn;

 cmdInsert.ExecuteNonQuery();

 Decimal newId = (Decimal)cmdInsert.ExecuteScalar();

 SqlCommand cmdInsert2 = new SqlCommand("INSERT INTO asset(n_asset_id,office_id,asset_status,asset_accountable,asset_remarks) VALUES ('" + newId + "','" + inputOffice + "','" + inputStatus + "','" + inputAccountable + "','" + inputRemarks + "'");

 cmdInsert2.CommandType = System.Data.CommandType.Text;
 cmdInsert2.Connection = conn;

 cmdInsert2.ExecuteNonQuery();

Then I want to get the appropriate id which is n_asset_id.

The problem is the cmdInsert is doubling its insert (maybe in my if condition) but I can't make the Decimal newId = (Decimal)cmdInsert2.ExecuteScalar(); to run correctly. I tried making it an Int. then string -> convert to int but still no luck.

Community
  • 1
  • 1
Fiendcoder1
  • 119
  • 1
  • 1
  • 12
  • 1
    How are you even executing `cmdInsert2` before you define it? If that's supposed to be `cmdInsert` then it's "doubling its insert" because, well, you're executing it twice. Also, what "if condition" are you referring to? – David Feb 07 '17 at 03:01
  • Im executing it right after `cmdInsert` then the suppose to be variable `should`/`must` be saved in the `newId` – Fiendcoder1 Feb 07 '17 at 03:04
  • I see that `Decimal newId = (Decimal)cmdInsert2.ExecuteScalar();` is placed above `cmdInsert2` declaration. You need to put the line *after* declaring `cmdInsert2` as `SqlCommand`. – Tetsuya Yamamoto Feb 07 '17 at 03:05
  • You're executing `cmdInsert2.ExecuteScalar()`, and then defining `SqlCommand cmdInsert2 = ...` on the very next line. The code you've posted doesn't compile. – David Feb 07 '17 at 03:05
  • Sorry wrong type. i've misplaced it while editing. There I've edited now – Fiendcoder1 Feb 07 '17 at 03:06
  • @Fiendcoder1: Ok, now... What's the specific question here? Are you asking why `cmdInsert` is executing twice? The answer is because you're executing it twice. On two consecutive lines of code right next to each other. – David Feb 07 '17 at 03:07
  • I don't see any consecutive same line of code to be next to each other. and also I just want to get the last inserted ID for that. :( – Fiendcoder1 Feb 07 '17 at 03:09
  • 1
    Do not concatenate parameters into your query. That's just begging for a SQL Injection Attack or for some value with an apostrophe or other special symbol to wreck your query. Use properly parameterized commands. See [Why do we always prefer using parameters in SQL statements?](http://stackoverflow.com/questions/7505808/why-do-we-always-prefer-using-parameters-in-sql-statements) – mason Feb 07 '17 at 03:11
  • The numbers mason, what do they mean. – Fiendcoder1 Feb 07 '17 at 03:16
  • I said to the question that I refer to that topic :) – Fiendcoder1 Feb 07 '17 at 06:13
  • I know. The referred question completely describes what you want to do, but your code looks nothing like it. – Nick.Mc Feb 07 '17 at 22:20

1 Answers1

3

The problem is the cmdInsert is doubling its insert

That's because you're executing the SQL query twice:

cmdInsert.ExecuteNonQuery();
Decimal newId = (Decimal)cmdInsert.ExecuteScalar();

If you just want the latter, you should omit the former entirely:

Decimal newId = (Decimal)cmdInsert.ExecuteScalar();

The output should probably also be an int, since the inserted identifier will be an int:

int newId = (int)cmdInsert.ExecuteScalar();

As an aside, be aware that your code is wide open to what's called "SQL injection attacks". That is, savvy users can cause your system to execute arbitrary SQL code on your server. This same vulnerability also makes your code more fragile and more difficult to debug, since unexpected characters (not necessarily malicious, even just accidental) can cause errors in your SQL code and you don't even know until runtime what your SQL code is.

To address this, look into using parameters in your queries. Never execute values as code, always add them to your command objects as parameter values.

David
  • 208,112
  • 36
  • 198
  • 279
  • I'll check that later, good sir. – Fiendcoder1 Feb 07 '17 at 03:10
  • `int newId = (int)cmdInsert.ExecuteScalar();` this will throw exception. Maybe like that `int newId = Convert.ToInt32(cmdInsert.ExecuteScalar());` – Sergio Feb 07 '17 at 04:06
  • I have followed your advice, though. Sergio's correct about it will throw exception. – Fiendcoder1 Feb 07 '17 at 05:59
  • 1. You haven't even come close to following the original question you referenced in your original question which perfectly explains what you need to do; 2. Please don't mark an answer as correct it doesn't do what you ask (returns the identity value) – Nick.Mc Feb 07 '17 at 06:03