Sql problems working with currency

[JHatman]

Ok basically I'm trying to store a monetary value in a sql database. In the database my field is "Price" and I've set it to a Real value. So far so good.

Thing thats confusing me is what to call the variable in asp.net, a String? An Int?

This is basically the code the runs the stored proc:

		Dim strSQL as string
		
		Dim strGCProductCode as string
		Dim strGCDescription as string
		Dim strGCPrice
		
		strGCProductCode = Trim(txtGCProductCode.Text)
		strGCDescription = Trim(txtGCDescription.Text)
		strGCPrice = Trim(txtGCPrice.Text)
			
		strSQL = "ProcUpdateGCDetails " & "'"
		strSQL = strSQL & strGCProductCode & "', '"
		strSQL = strSQL & strGCDescription & "', '"
		strSQL = strSQL & strGCPrice & "', "
		strSQL = strSQL & cInt(Request.QueryString("ProductID"))

And this is the stored proc code:

CREATE PROCEDURE [dbo].[ProcUpdateGCDetails] 
(
@strGCProductCode Varchar(50),
@strGCDescription Varchar(50),
@ProductID Int,
@strGCPrice Smallmoney)

AS UPDATE		dbo.tblGCProducts
SET				ProductCode=@strGCProductCode,
				Description=@strGCDescription,
				Price=@strGCPrice
WHERE			(ProductID = @ProductID)

However, when I'm trying to update, this is the error I'm getting and for the likes of me I just cannot figure it out:

http://img25.imageshack.us/i/errorew.jpg/

needing some input pls guys!

 

[FarligOpptreden]

I'm assuming you're using SQL Server, right? Why not store the data as type Money? Also, in .NET the best option would be to interpret Price as type Decimal. Always strongly type your variables. With a string you will have hundreds of type-conversion errors.

EDIT 1: When executing the Stored Procedure, why are you concatenating a string? That opens up another host of problems. Use the objects in the System.Data.SqlClient namespace? That allows you to specify an array of SqlParameter objects when executing your stored procedure, in the vein of:

// Initialitze your SqlCommand object with the name of the Stored Procedure and the Connection object.
SqlCommand command = new SqlCommand(strStoredProcedureName, connectionObject);
// Set the CommandType of the Command to Stored Procedure.
command.CommandType = CommandType.StoredProcedure;

// Create an array of SqlParameter objects and initialize the array with all your parameters.
SqlParameter[] params = new SqlParameter[]
    {
        new SqlParameter("@strGCProductCode", strGCProductCode),
        new SqlParameter("@strGCDescription ", strGCDescription),
        new SqlParameter("@ProductID", Request.QueryString("ProductID")),
        new SqlParameter("@strGCPrice", strGCPrice)
    };
// Add each parameter in the array to the Command object's Parameters collection.
foreach (SqlParameter param in params)
{
    command.Parameters.Add(param);
}

// Execute the stored procedure.
command.ExecuteNonQuery();
// Close the connection to the database. VERY IMPORTANT!
connectionObject.Close();

EDIT 2: I see you're using VB.NET. Just follow the link in the first post of the .NET Knowledge Sharing Thread to convert the C# to VB. Link to the thread is in my sig and it has been stickied in the Software / Web Development forum as well.

 

[guest2013-1]

Edited version:

I just saw the screenshot you added.

Change:

strSQL = strSQL & cInt(Request.QueryString("ProductID"))

To:

strSQL = strSQL & Request.QueryString("ProductID")

Remember, if you're doing it like you're doing it with building the SQL String like you are, you need everything to be text (String). I would highly recommend using SqlCommand instead

The error does indicate that something went wrong within your stored procedure though, but I would take a stab and fix the code first and debug the stored procedure separately using Query Analyzer

Old Post:

I'm assuming you're using SQL Server, right? Why not store the data as type Money? Also, in .NET the best option would be to interpret Price as type Decimal. Always strongly type your variables. With a string you will have hundreds of type-conversion errors.

Your assumption is incorrect. He set his "Price" Column to have a "Real" value. Which indicates he is using MS Access.

To the op. You can declare this either As Double or As Float. Int won't preserve the possible decimals on the amount you enter as it would just throw them away. With declaring it as a String you won't be able to do any calculations without converting it to either Double/Float or Int to begin with......

If you're teaching yourself, awesome. If not, get your money back.

 

[dequadin]

As FarligOpptreden said decimal type is the best in .NET for storing money. You can confirm this on MSDN.

Two problems with your code:
1. You stored procedure parameters go:
@strGCProductCode Varchar(50),
@strGCDescription Varchar(50),
@ProductID Int,
@strGCPrice Smallmoney)

You are passing it
strSQL = strSQL & strGCProductCode & "', '"
strSQL = strSQL & strGCDescription & "', '"
strSQL = strSQL & strGCPrice & "', "
strSQL = strSQL & cInt(Request.QueryString("ProductID")

Product ID and Price are swapped around.

2. Why aren't you using SQLCommand.CommandType.StoredProcedure and then adding your parameters using mySQLCommand.AddParameter?

Look here and here to see what I'm talking about.

 

[guest2013-1]

Found the reason why you're erroring there. (I mentioned the error you have there is SQL Server based and not code based)

strSQL = "ProcUpdateGCDetails " & "'"
		strSQL = strSQL & strGCProductCode & "', '"
		strSQL = strSQL & strGCDescription & "', '"
		strSQL = strSQL & strGCPrice & "', "
		strSQL = strSQL & Request.QueryString("ProductID")

You pass 4 variables to the stored procedure, in order: Code/Description/Price/ProductID

However:

CREATE PROCEDURE [dbo].[ProcUpdateGCDetails] 
(
@strGCProductCode Varchar(50),
@strGCDescription Varchar(50),
@ProductID Int,
@strGCPrice Smallmoney)

Your stored procedure accepts the parameters in the order: Code/Description/ProductID/Price

So effectively you're passing a string value to an integer only field in your stored procedure.

 

[JHatman]

Wow given me some pretty concrete pointers there guys, really helpfull thanks!! Busy working through them now, have changed the value in SQL to Float, and gonna try use the System.Data.SqlClient namespace to clean it up a little.

Thanks dequadin and Acid, you know I was staring at the code for ages looking to see if I got them in the right order when passing it into the stored proc and it just looked right, just shows another perspective is a huge help

Will post results once I've made the changes!

 

[FarligOpptreden]

I updated my post. Rather use an SqlParameter[] than concatenating a string... That way you don't need to pass it in a specific order.

@Acid: The error stems from the System.Data.SqlClient namespace, so I'm assuming it IS SQL Server...

 

[guest2013-1]

Wow given me some pretty concrete pointers there guys, really helpfull thanks!! Busy working through them now, have changed the value in SQL to Float, and gonna try use the System.Data.SqlClient namespace to clean it up a little.

Thanks dequadin and Acid, you know I was staring at the code for ages looking to see if I got them in the right order when passing it into the stored proc and it just looked right, just shows another perspective is a huge help

Will post results once I've made the changes!

Yep, test in Query Analyzer first. Then your code. And remember this nugget:

If the ASP.NET error talks about varchar, it's SQL Server related. So there's either something wrong with your code passing the wrong values to your stored procedure, or something is ****ing out in your stored procedure.

ASP.NET talks about String, not varchar.

 

[FarligOpptreden]

2. Why aren't you using SQLCommand.CommandType.StoredProcedure and then adding your parameters using mySQLCommand.AddParameter?

Look here and here to see what I'm talking about.

...as I stated in my post as well. And the post following that one.

 

[dequadin]

...as I stated in my post as well. And the post following that one.

Yeah after all of you guys scrambled to eDiT

One improvement on your code would be:

using (SqlConnection sqlConnection = new SqlConnection(sqlConnectionStringBuilder.ToString());
{
    // Insert all of FarligOpptreden's code here except the last line
    // connectionObject.Close();
    // and replace connectionObject with sqlConnection 
}

I pimped my signature too!

 

[FarligOpptreden]

Yeah after all of you guys scrambled to eDiT

Na-ahhh... I was busy editing and when I submitted... BAM. Tons of new posts!

One improvement on your code would be:

using (SqlConnection sqlConnection = new SqlConnection(sqlConnectionStringBuilder.ToString());
{
    // Insert all of FarligOpptreden's code here except the last line
    // connectionObject.Close();
    // and replace connectionObject with sqlConnection 
}

Of course. I just wanted to explain the principles. The better option would be to have a base class handling all your database interactions and extend it for the different database entities, protecting the methods that execute queries against the database. After all, you don't want any misguided database queries in an enterprise application, now do you?

I pimped my signature too!

Nicely done! We need to spread the word brother! AMEN and HALLELUJAH!

 

[dequadin]

Na-ahhh... I was busy editing and when I submitted... BAM. Tons of new posts!

lol, if you say so Have you seen how they handle that on StackOverflow. If you're busy ans someone else answers they put a banner type thing at the top informing you that there are new answers. It works well.

Of course. I just wanted to explain the principles. The better option would be to have a base class handling all your database interactions and extend it for the different database entities, protecting the methods that execute queries against the database. After all, you don't want any misguided database queries in an enterprise application, now do you?

For sure having separate layers is the ultimate best way to go. But the using keyword isn't explained much on the net. I've hardly seen it in any articles on this type of thing, and it works so well!

 

[FarligOpptreden]

For sure having separate layers is the ultimate best way to go. But the using keyword isn't explained much on the net. I've hardly seen it in any articles on this type of thing, and it works so well!

UPFRONT DISCLAIMER: An edit was performed on this post, so regard its content higher than any posts after this.

Yeah, the "using" keyword is the C# alternative to VB.NET's "with". But somehow I still prefer typing everything out... It just adds to the readability of the code. But each to their own I suppose?

EDIT: That might be a good topic for the Knowledge Sharing Thread. Make a detailed post discussing the use of the "using" keyword and how to properly utilize it.

 

[dequadin]

UPFRONT DISCLAIMER: An edit was performed on this post, so regard its content higher than any posts after this.

Yeah, the "using" keyword is the C# alternative to VB.NET's "with". But somehow I still prefer typing everything out... It just adds to the readability of the code. But each to their own I suppose?

EDIT: That might be a good topic for the Knowledge Sharing Thread. Make a detailed post discussing the use of the "using" keyword and how to properly utilize it.

That was awesome edit timing, as I clicked quote you edit appeared +1

using is definitelynot the same as with.

with allows you to access properties of an object like this

With myObect
   .Property1
   .Property2
End With

Which is equivalent to:
myObect.Property1
myObect.Property2

That's not what using does. As per MDSN on the using keyword

using
Provides a convenient syntax that ensures the correct use of IDisposable objects.

As a rule, when you use an IDisposable object, you should declare and instantiate it in a using statement. The using statement calls the Dispose method on the object in the correct way, and (when you use it as shown earlier) it also causes the object itself to go out of scope as soon as Dispose is called. Within the using block, the object is read-only and cannot be modified or reassigned.

The using statement ensures that Dispose is called even if an exception occurs while you are calling methods on the object. You can achieve the same result by putting the object inside a try block and then calling Dispose in a finally block; in fact, this is how the using statement is translated by the compiler.

It has an equivalent syntax in VB.NET, and an SqlConnection is an excellent candidate to be wrapped in a using block.

EDIT: Yes the knowledge sharing thread needs some info on this...

 

[guest2013-1]

c# = using
vb.net = imports

 

[dequadin]

c# = using
vb.net = imports

No! That's for importing namespaces...

C#

using (SomeClass myObject = new SomeClass())
{
  // You can access myObject anywhere here
}

VB.NET

Using(SomeClass myObject As New SomeClass())
  ' You can access myObject anywhere here
End Using

 

[FarligOpptreden]

Hmmm... I think I misunderstood the using / with thing then somewhere. Guess I never really gave much attention to the "using" keyword. It actually seems like it might have some use for me after all...