I am making a web store in ASP.NET with a SQL Server database. I’m currently working on a "edit users" part of control panel. I have list of all users and "edit" and "delete" buttons for each row of a list. When edit button is clicked I put mail of a clicked user in Session["EditMail"] and redirect to edituser.aspx where I load user’s data in textboxes and his role in dropdown. In the bottom is "save changes" button. When button is clicked it activate this method:
protected void btnSaveChanges_Click(object sender, EventArgs e)
{
string name = txtName.Text;
string surname = txtSurname.Text;
string username = txtUsername.Text;
string email = txtEmail.Text;
string password = txtPassword.Text;
string country = txtCountry.Text;
string city = txtCity.Text;
int postCode = Convert.ToInt32(txtPostCode.Text);
string address = txtAddress.Text;
int user_role = Convert.ToInt32(DropDownListRole.SelectedItem.Value);
WebShop moj_nalog1 = new WebShop();
int izmena_korisnika = moj_nalog1.Izmena_Korisnika(name, surname, username, password, mail, country, city, postCode, address, user_role);
if (izmena_korisnika == 0)
{
Session["EditMail"] = email;
Response.Redirect("edituser.aspx");
}
else
{
lblError.Visible = false;
lblError.Text = "Error!";
lblError.CssClass = "text-danger";
lblError.Visible = true;
}
}
This is Izmena_Korisnika (Edit user in english) method in WebShop.cs:
public int Izmena_Korisnika(string name, string surname, string username, string password, string email, string country, string city, int post_code, string address, int role_id)
{
conn.ConnectionString = wqbConfig;
int result;
comm.Connection = conn;
comm.CommandType = CommandType.StoredProcedure;
comm.CommandText = "dbo.Korisnik_Izmeni";
// kolekcija Parameters
comm.Parameters.Add(new SqlParameter("@ime", SqlDbType.NVarChar, 100, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, name));
comm.Parameters.Add(new SqlParameter("@prezime", SqlDbType.NVarChar, 100, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, surname));
comm.Parameters.Add(new SqlParameter("@username", SqlDbType.NVarChar, 30, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, username));
comm.Parameters.Add(new SqlParameter("@lozinka", SqlDbType.NVarChar, 255, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, password));
comm.Parameters.Add(new SqlParameter("@email", SqlDbType.NVarChar, 50, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, email));
comm.Parameters.Add(new SqlParameter("@drzava", SqlDbType.NVarChar, 100, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, country));
comm.Parameters.Add(new SqlParameter("@grad", SqlDbType.NVarChar, 100, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, city));
comm.Parameters.Add(new SqlParameter("@postanski_br", SqlDbType.Int, 5, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, post_code));
comm.Parameters.Add(new SqlParameter("@adresa", SqlDbType.NVarChar, 255, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, address));
comm.Parameters.Add(new SqlParameter("@uloga_korisnika_id", SqlDbType.Int, 4, ParameterDirection.Input, false, 0, 0, "", DataRowVersion.Current, role_id));
comm.Parameters.Add(new SqlParameter("@RETURN_VALUE", SqlDbType.Int, 4, ParameterDirection.ReturnValue, true, 0, 0, "", DataRowVersion.Current, null));
conn.Open();
comm.ExecuteNonQuery();
conn.Close();
int Ret;
Ret = (int)comm.Parameters["@RETURN_VALUE"].Value;
if (Ret == 0)
{
result = 0;
}
else
{
result = 1;
}
return result;
}
And this is the SQL Server stored procedure dbo.Korisnik_Izmeni:
CREATE PROCEDURE Korisnik_Izmeni
@ime nvarchar(100),
@prezime nvarchar(100),
@username nvarchar(30),
@lozinka nvarchar(255),
@email nvarchar(50),
@drzava nvarchar(100),
@grad nvarchar(100),
@postanski_br int,
@adresa nvarchar(255),
@uloga_korisnika_id int
AS
SET LOCK_TIMEOUT 3000;
BEGIN TRY
IF EXISTS (SELECT TOP 1 ime FROM Korisnici
WHERE email = @email)
BEGIN
UPDATE Korisnici
SET ime = @ime,
prezime = @prezime,
username = @username,
lozinka = @lozinka,
drzava = @drzava,
grad = @grad,
postanski_br = @postanski_br,
adresa = @adresa,
uloga_korisnika_id = @uloga_korisnika_id
WHERE email = @email
RETURN 0;
END
RETURN -1;
END TRY
BEGIN CATCH
RETURN @@ERROR;
END CATCH
When I click the button data does not update. No error or exception is shown, lblError also does not become visible which means that izmena_korisnika in btnSaveChanges_Click is equal to 0. How is it then possible that data does not update
I have almost same code (minus dropdown for role) in myaccount.aspx page that is also for editing user’s data. It works there but do not work in edituser.aspx.
2
Answers
In both TSQL and .NET DO NOT use return values to indicate success or failure. Use Exceptions in .NET and Errors (THROW/RAISERROR) in TSQL.
And even if the stored procedure returns a non-zero value on error, you should never rely on stored procedure return values to determine success or failure. Instead throw errors, and determine success by the lack of an error. The risk that the client won’t correctly interpret the return value is too high.
Using stored procedure return values to determine success or failure is an old pattern that predates TSQL TRY/CATCH, and was never recommended from client code. So just THROW in the CATCH block or omit the TRY/CATCH entirely.
Ok, you starting to create a "mess" here.
Let’s clean this up.
Try say like this: