patternsqlMinor
Small query - removing XML tags
Viewed 0 times
removingqueryxmltagssmall
Problem
I have some code that I wrote almost a year ago exactly (1/9/2013) and I would like to know if I wrote it well or if it can be improved. I don't have any fun input or output, as these are not set results coming out of this column in the table.
I cannot change the tables. Here are the table layouts:
uCode:
```
CREATE TABLE [dbo].uCodeWITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LO
-- =============================================
-- Author: Malachi (Name Changed to Protect the possibly Guilty)
-- Create date: 1/9/2013
-- Description: Will give a list of the Bond Conditions
-- =============================================
CREATE FUNCTION [dbo].[fnBondConditionList]
(
@BondID int
)
RETURNS Varchar(MAX)
AS
BEGIN
DECLARE @Result Varchar(MAX)
SELECT @Result = (Select Justice.dbo.uCode.Description as BondConditions
FROM Justice.dbo.uCode INNER JOIN
Justice.dbo.xBondCondition ON Justice.dbo.uCode.CodeID = Justice.dbo.xBondCondition.ConditionID
WHERE Justice.dbo.xBondCondition.BondID = @BondID
FOR XML PATH (''))
SET @Result = replace(@Result, '','')
SET @Result = replace(@Result, '','')
Set @Result = LEFT(@Result, LEN(@Result) - 1)
RETURN @Result
ENDI cannot change the tables. Here are the table layouts:
uCode:
```
CREATE TABLE [dbo].uCodeWITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LO
Solution
If you do not want the XML Tags, then do not add them!
This code frustrates me because:
, and will thus leave some invalid X?HTML? I presume? A bug?
As for some nit-picks:
As far as I am concerned, this function should be removed entirely, and a simple presentation-layer system should be added that simply appends a
This code frustrates me because:
- you have not given any indication as to what this code should be doing.
- you are embedding presentation-layer logic in the database layer
- you convert all the data to an XML document, and then strip all the XML tags from the result because this appears to be a 'cheap' way to get all the data as a single result instead of multiple rows.
- you do not document this
- you use less-common features of the database (XML manipulation) without good reason, which locks you in to a single provider for the database ;-)
- there is no documented reason for why you strip the last character from the
@result... this will strip the closing>from the last
, and will thus leave some invalid X?HTML? I presume? A bug?
As for some nit-picks:
- You should not be using
SELECT @Result = (...), but should, instead be usingSET @Result = (...)See the documentation for SELECT.
- you capitalize some of your query key-words, but not all. I recommend using whatever case you do not use for your columns. In your case, your db.schema.table.columns are all lower/CamelCase, so your keywords should all be full CAPITALS. Thus, you should have
SELECTeverywhere. You have oneSELECT, and oneSelect. You have oneLEFT, and tworeplace. Your consistency of convention should be improved.
- you do not use table-aliases for your query objects. Not only do table-aliases make the SQL more readable, but, if you need to change a table name for some reason (e.g. to get data from a view, or different schema), then you have to change multiple places, and could lead to bugs....
As far as I am concerned, this function should be removed entirely, and a simple presentation-layer system should be added that simply appends a
after each bond description from the query:SELECT uC.Description
FROM Justice.dbo.uCode uC
INNER JOIN Justice.dbo.xBondCondition xBC
ON uC.CodeID = xBC.ConditionID
WHERE xBC.BondID = ?Code Snippets
SELECT uC.Description
FROM Justice.dbo.uCode uC
INNER JOIN Justice.dbo.xBondCondition xBC
ON uC.CodeID = xBC.ConditionID
WHERE xBC.BondID = ?Context
StackExchange Code Review Q#38444, answer score: 8
Revisions (0)
No revisions yet.