May 1, 2020 at 7:25 pm
Many examples on the web, eg. sqlauthority show comment block before the CREATE AS
https://www.mssqltips.com/sqlservertutorial/167/using-comments-in-a-sql-server-stored-procedure/
Is this correct, or is there an argument for placing it after AS? Why?
notes: by comment block I mean this thing that we put at the top of a sproc to describe it and track modifications
/*
-this procedure gets a list of addresses based
on the city value that is passed
-this procedure is used by the HR system
*/
--Quote me
May 1, 2020 at 7:52 pm
it is a matter of opinion.
on my own opinion I enforce them to be after the create.
one of the reasons is that if I ever need to do a automated change of the code ensuring that the create proc and the proc name are at the very start is a rather good thing (and speaking for experience as I have done this recently)
one other reason - I do really hate the fact that MS treats things that are OUTSIDE the create block as being part of it.
May 1, 2020 at 8:56 pm
Frederico,
on the other post you suggested my code would actually FAIL "if you are one of those that puts comments BEFORE the create procedure command"
So now you are saying 'it's a matter of opinion'???
I would like to see additional members pitch in their KT on this one please.
--Quote me
May 1, 2020 at 9:11 pm
My preference is before partly because that is what was done before I started and partly because that is my habit from .NET development.
From a C# side, my opinion is that comment blocks should not exist inside of a function. If you need a whole comment block to describe a subset of a function, then that subset should be its own function. A comment line may exist in the function, but blocks of comments mean the code is too complicated or your comment should be simplicied.
For consistency sake, we like having our code with comments before the function to describe the function in C# and follow that pattern with tables, views, stored procedures and functions in SQL.
That is just the rule we have at work, but it doesn't mean it is some universal rule. Everything I've seen seems to indicate this is a matter of opinion, but once you pick one method, you should stick to it.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
May 1, 2020 at 10:00 pm
Frederico,
on the other post you suggested my code would actually FAIL "if you are one of those that puts comments BEFORE the create procedure command"
So now you are saying 'it's a matter of opinion'???
I would like to see additional members pitch in their KT on this one please.
yes it is a matter of opinion
lets put this as an example.your code to change the SP does the following replacement (and making the assumption that you change your code to look at the first occurrence of CREATE to replace with ALTER
/* this proc is to create a copy of a table and alter its indexes for performance
for all intents and purposes this is an alter procedure type
*/
create procedure procname
as
begin
if object_id('tempdb..#xxx') is not null
drop table #xxx;
create table #xxx
(id int
,description varchar(200)
);
alter table #xxx add constraint pk_xxx primary key nonclustered
(id
);
end
when you do your your code (which by the way is incorrect as
select definition
from sys.sql_modules
returns a "CREATE PROC..." and not an ALTER PROC so your code
SELECT
Replace ((REPLACE(definition,'subscriptonguid','subscriptionguid')),'ALTER','create')
FROM sys.sql_modules a
isn't even doing what you intended.
in any case on the sample proc above your code would return the following (words in BOLD are the ones your code replaces)
/* this proc is to create a copy of a table and alter its indexes for performance
for all intents and purposes this is an CREATE procedure type
*/
create procedure procname
as
begin
if object_id('tempdb..#xxx') is not null
drop table #xxx;
create table #xxx
(id int
,description varchar(200)
);
CREATE table #xxx add constraint pk_xxx primary key nonclustered
(id
);
end
as you did a drop of the proc the create works - but you loose all grants on it and have to do them again
so when you try and recreate the proc above it will fail as your add primary constraint got changed to an invalid CREATE table statement
had you used the correct replacement of CREATE by ALTER (so you could keep the grants)
then the code would be
/* this proc is to create a copy of a table and alter its indexes for performance
for all intents and purposes this is an CREATE procedure type
*/
ALTER procedure procname
as
begin
if object_id('tempdb..#xxx') is not null
drop table #xxx;
ALTER table #xxx
(id int
,description varchar(200)
);
alter table #xxx add constraint pk_xxx primary key nonclustered
(id
);
end
this time it fails because your code is trying to issue an invalid ALTER table statement
had you searched for the first "create" to replace with an "ALTER" again it would fail because it would find the "CREATE procedure" within the comments - had the comments been after the create proc this would not be an issue.
so as you see there are lots of things to take in consideration - a few more even but no point in stating them as it is the wrong thing to do for a simple replacement that should be done on your source code, not directly on the database.
and apologies - my "will fail" should have been "may fail" - not that it matters
August 18, 2022 at 1:15 am
Hello, you have a question. If you need the entire block you like to describe a set with one function, then the set with that must be its own function.
August 18, 2022 at 2:36 pm
Put comments inside the code, not before it.
frederico_fonseca stated why:
(1) it's a royal pain trying to make automated changes if comments precede the CREATE / ALTER
(2) it's odd to consider code before an object to be part of the object.
Also:
(A) Put only comments necessary to describe and explain the proc at the start of the proc. For example, some people love to have a code history of changes; fine, but put that at the end of the code, since that offers no understanding of the code.
(B) Declare all variables and temp tables at the start of the proc, variables first (in alpha order!), then temp tables (in alpha order!).
(C) Don't use a comment in place of an accurate variable name. That should go without saying, but I've seen it a million times:
DECLARE @counter int; --number of files to process --NO, NO!, NO!!
DECLARE @files_to_process_count int; --there ya go
SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".
Viewing 7 posts - 1 through 6 (of 6 total)
You must be logged in to reply to this topic. Login to reply