patternsqlMinor
Saving and updating records
Viewed 0 times
andupdatingsavingrecords
Problem
I have made two SQL statements as I was unable to figure out how to solve my problem. One is called
Product_GetID and the other Product_SaveRecord. I wanted the save record to read and see if there was an ID entered in a field in a VB front end and if there was not, it would assign one for it, but I was unable to figure this out. It works, but I know there should be a better way to do this.ALTER PROCEDURE [dbo].[Product_GetID]
AS
BEGIN
DECLARE @NewProductID as int
/* Get New Record */
SET @NewProductID = (SELECT ISNULL(MAX(ProductID) + 1, 1) FROM Product)
SELECT ISNULL(MAX(ProductID) + 1, 1) AS NewIDNo FROM Product
INSERT INTO Product (ProductID, CategoryID, Description, Price)
VALUES (@NewProductID, 0, '', 0)
END
ALTER PROCEDURE [dbo].[Product_SaveRecord]
-- Add the parameters for the stored procedure here
@ProductID as int, @CategoryID as tinyint, @Description as varchar(80), @FullDescription as varchar(240), @Price as decimal(8, 2), @MarkupPer as decimal(8, 2), @LabourHours as decimal(8, 2), @LabourRate as decimal(8, 2), @Stock as integer
AS
BEGIN
/* Update Record */
UPDATE Product SET CategoryID = @CategoryID,
Description = @Description,
FullDescription = @FullDescription,
Price = @Price,
MarkupPer = @MarkupPer,
LabourHours = @LabourHours,
LabourRate = @LabourRate,
Stock = @Stock
WHERE ProductID = @ProductID
ENDSolution
Nitpicks
Why is this indented?
This indentation is inconsistent:
The comments are just noise, it's pretty obvious from the way the code reads to figure out what it does. Especially comments like this:
Vertical white space would make your code easier to read. For example:
Compare to:
Also, consistency would be good in how you capitalize keywords. Either do all caps, or all lower caps; it's less confusing that way.
End your SQL statements
Not using the delimiter
Your question
You're trying to insert a row and then use the ID of the row you inserted. There is an easier way to do this using
Here would be a simpler way to do this your way, with above advice applied:
BUT...
Why do you not simply insert your parameters within one single
Why is this indented?
ALTER PROCEDURE [dbo].[Product_GetID]This indentation is inconsistent:
UPDATE Product SET CategoryID = @CategoryID,
Description = @Description,
FullDescription = @FullDescription,
Price = @Price,
MarkupPer = @MarkupPer,
LabourHours = @LabourHours,
LabourRate = @LabourRate,
Stock = @Stock
WHERE ProductID = @ProductID
ENDThe comments are just noise, it's pretty obvious from the way the code reads to figure out what it does. Especially comments like this:
/* Update Record */
UPDATE Product SET CategoryID = @CategoryID,Vertical white space would make your code easier to read. For example:
ALTER PROCEDURE [dbo].[Product_SaveRecord]
@ProductID as int, @CategoryID as tinyint, @Description as varchar(80), @FullDescription as varchar(240), @Price as decimal(8, 2), @MarkupPer as decimal(8, 2), @LabourHours as decimal(8, 2), @LabourRate as decimal(8, 2), @Stock as integerCompare to:
ALTER PROCEDURE [dbo].[Product_SaveRecord]
@ProductID as int,
@CategoryID as tinyint,
@Description as varchar(80),
@FullDescription as varchar(240),
@Price as decimal(8, 2),
@MarkupPer as decimal(8, 2),
@LabourHours as decimal(8, 2),
@LabourRate as decimal(8, 2),
@Stock as integer;Also, consistency would be good in how you capitalize keywords. Either do all caps, or all lower caps; it's less confusing that way.
End your SQL statements
Not using the delimiter
; throughout your query makes it more difficult to understand the logic of your code. Use ; and even GO if need be. MS SQL Server is forgiving on syntax errors (try PostgreSQL for fun), that doesn't mean you shouldn't follow good practices of ending statements properly. Your question
You're trying to insert a row and then use the ID of the row you inserted. There is an easier way to do this using
@@IDENTITY in Transact-SQL.Here would be a simpler way to do this your way, with above advice applied:
ALTER PROCEDURE [dbo].[Product_GetID]
@ProductID AS INT,
@CategoryID AS TINYINT,
@Description AS VARCHAR(80),
@FullDescription AS VARCHAR(240),
@Price AS DECIMAL(8, 2),
@MarkupPer AS DECIMAL(8, 2),
@LabourHours AS DECIMAL(8, 2),
@LabourRate AS DECIMAL(8, 2),
@Stock AS INTEGER
AS
BEGIN
INSERT INTO Product (CategoryID, Description, Price)
VALUES (0, '', 0);
UPDATE Product
SET CategoryID = @CategoryID,
Description = @Description,
FullDescription = @FullDescription,
Price = @Price,
MarkupPer = @MarkupPer,
LabourHours = @LabourHours,
LabourRate = @LabourRate,
Stock = @Stock
WHERE ProductID = @@IDENTITY;
ENDBUT...
Why do you not simply insert your parameters within one single
INSERT transaction?ALTER PROCEDURE [dbo].[Product_GetID]
@ProductID AS INT,
@CategoryID AS TINYINT,
@Description AS VARCHAR(80),
@FullDescription AS VARCHAR(240),
@Price AS DECIMAL(8, 2),
@MarkupPer AS DECIMAL(8, 2),
@LabourHours AS DECIMAL(8, 2),
@LabourRate AS DECIMAL(8, 2),
@Stock AS INTEGER
AS
BEGIN
INSERT INTO Product(
CategoryID,
Description,
FullDescription,
Price,
MarkupPer,
LabourHours,
LabourRate,
Stock
) VALUES (
@CategoryID,
@Description,
@FullDescription,
@Price,
@MarkupPer,
@LabourHours,
@LabourRate,
@Stock
);
ENDCode Snippets
ALTER PROCEDURE [dbo].[Product_GetID]UPDATE Product SET CategoryID = @CategoryID,
Description = @Description,
FullDescription = @FullDescription,
Price = @Price,
MarkupPer = @MarkupPer,
LabourHours = @LabourHours,
LabourRate = @LabourRate,
Stock = @Stock
WHERE ProductID = @ProductID
END/* Update Record */
UPDATE Product SET CategoryID = @CategoryID,ALTER PROCEDURE [dbo].[Product_SaveRecord]
@ProductID as int, @CategoryID as tinyint, @Description as varchar(80), @FullDescription as varchar(240), @Price as decimal(8, 2), @MarkupPer as decimal(8, 2), @LabourHours as decimal(8, 2), @LabourRate as decimal(8, 2), @Stock as integerALTER PROCEDURE [dbo].[Product_SaveRecord]
@ProductID as int,
@CategoryID as tinyint,
@Description as varchar(80),
@FullDescription as varchar(240),
@Price as decimal(8, 2),
@MarkupPer as decimal(8, 2),
@LabourHours as decimal(8, 2),
@LabourRate as decimal(8, 2),
@Stock as integer;Context
StackExchange Code Review Q#63765, answer score: 7
Revisions (0)
No revisions yet.