Silly Question

Question

Post reply

Silly Question

lenardd

Default port

Points: 1435
More actions
July 22, 2003 at 5:24 am

#159818

Can the following SP be better written? There has to be!
CREATE PROCEDURE dbo.spRef_GetCertifications
(
@activesOnly bit
)
AS
IF( @activesOnly = 1 )
SELECT ID, Name, IsActive, SubmittedOnGMT FROM Certification WHERE IsActive = 1 ORDER BY Name ASC
ELSE
SELECT ID, Name, IsActive, SubmittedOnGMT FROM Certification ORDER BY Name ASC
GO

Viewing 15 posts - 1 through 15 (of 16 total)

You must be logged in to reply to this topic. Login to reply

MarkusB SSC-Dedicated Points: 37370 More actions · Answer 1

This should work:

CREATE PROCEDURE dbo.spRef_GetCertifications

(

@activesOnly bit

)

AS

SELECT ID, Name, IsActive, SubmittedOnGMT

FROM Certification

WHERE IsActive = @activesOnly

ORDER BY Name ASC

[font="Verdana"]Markus Bohse[/font]

David Burrows SSC Guru Points: 65142 More actions · Answer 2

Why better, I see nothing wrong with your way. Another way of writing it would be

 CREATE PROCEDURE dbo.spRef_GetCertifications

(

@activesOnly bit

)

AS

SELECT ID, Name, IsActive, SubmittedOnGMT 

FROM Certification 

WHERE (@activesOnly = 0) OR (@activesOnly = 1 AND IsActive = 1)

ORDER BY Name ASC

GO

But I doubt there would be a great improvemnt in performance. Others may disagree.

Far away is close at hand in the images of elsewhere.
Anon.

Frank Kalis SSC Guru Points: 111183 More actions · Answer 3

Hi David,

quote:
Why better, I see nothing wrong with your way.

in Markus solution only one comparison is made, while you make 2 comparisons in your WHERE clause. I guess it's also for better readability

quote:
But I doubt there would be a great improvemnt in performance. Others may disagree.

Every little bit counts . As lenardd uses a bit variable, there can only be two stati. Either is a someone active, or not.

I would write it this way



CREATE PROCEDURE dbo.spRef_GetCertifications

(

@activesOnly bit = 0

)

AS

SELECT ID, Name, IsActive, SubmittedOnGMT 

FROM Certification 

WHERE IsActive = @activesOnly

ORDER BY Name ASC

making it a default of inactive

just my 0.02

Cheers,

Frank

--
Frank Kalis
Microsoft SQL Server MVP
Webmaster: http://www.insidesql.org/blogs
My blog: http://www.insidesql.org/blogs/frankkalis/[/url]

David Burrows SSC Guru Points: 65142 More actions · Answer 4

Hi Frank,

Actually three, 0, 1 and NULL. In the original and my query there is no provision for NULL and therefore i assumed (there's that word again) there were no NULLs.

I think your solution (as is MarkusB's) does not do what the original does. The original selects ALL (both inactive and active) records OR only active (IsActive=1).

There is no reference to what datatype IsActive is and I assumed (oh boy!) bit.

But I still stand by my first statement that the original is OK, will work and any other solution will not necessarily improve performance. I'm still waiting for someone to contradict this

Far away is close at hand in the images of elsewhere.
Anon.

Frank Kalis SSC Guru Points: 111183 More actions · Answer 5

quote:
Actually three, 0, 1 and NULL. In the original and my query there is no provision for NULL and therefore i assumed (there's that word again) there were no NULLs.

d*mn, you know there a three kinds of mathematicians.

One can count to three, the other can not.

quote:
I think your solution (as is MarkusB's) does not do what the original does. The original selects ALL (both inactive and active) records OR only active (IsActive=1).

hm... good point. Keep thinking on this.

quote:
But I still stand by my first statement that the original is OK, will work and any other solution will not necessarily improve performance. I'm still waiting for someone to contradict this

sure it will work.

Maybe it's all about writing nice looking sql

Cheers,

Frank

Edited by - a5xo3z1 on 07/23/2003 01:02:49 AM

--
Frank Kalis
Microsoft SQL Server MVP
Webmaster: http://www.insidesql.org/blogs
My blog: http://www.insidesql.org/blogs/frankkalis/[/url]

David Burrows SSC Guru Points: 65142 More actions · Answer 6

Hi Frank,

'writing nice looking sql'

Sure is, I try to make my code as tabular and indented as possible to make reading and understanding easy and especially for the next poor b*****d who has to read it. I all for slightly longer code rather than a compact complex mess that is hard to read as long as performance is acceptable.

Far away is close at hand in the images of elsewhere.
Anon.

Frank Kalis SSC Guru Points: 111183 More actions · Answer 7

quote:
Sure is, I try to make my code as tabular and indented as possible to make reading and understanding easy and especially for the next poor b*****d who has to read it. I all for slightly longer code rather than a compact complex mess that is hard to read as long as performance is acceptable.

that's very kind with respect to your colleagues. I myself take more care of me as to coding, when reviewing older code for improvements.

Hey, coding style is a candidate for another ~100,000 responses thread.

Cheers,

Frank

--
Frank Kalis
Microsoft SQL Server MVP
Webmaster: http://www.insidesql.org/blogs
My blog: http://www.insidesql.org/blogs/frankkalis/[/url]

lenardd Default port Points: 1435 More actions · Answer 8

Sorry, I forgot to mention that IsActive is a bit field and allows for NO nulls.

MarkusB, your way is not the same thing because yours returns "actives" OR "inactives".

What I was trying to do is return either "actives" OR "actives and inactives".

DavidBurrows, you understood correctly. The reason I was looking for something better is because some of my selects are much more complicated and just wanted to see if there was a way to short circuit the ELSE part all into the IF part without affecting performance. I think your way has better readablity for my complicated SELECTS.

I posted because, I thought there was a way to do it like much like when you create dynamic SELECT statement in a programming language...for example, one way I would have approached it when creating the SELECT clause in C# would have been:

 

string s = "SELECT ID, Name, IsActive FROM Certification";

If( activesOnly ) s =+ " WHERE IsActive=1 ";

s =+ "Order By Name ASC";

Or another way would have been:

 

string s = "SELECT ID, Name, IsActive FROM Certification {0} Order By Name ASC";

s = string.Format( s, ( activesOnly == true ? "WHERE IsActive=1" : "" ) );

Anyways, thanks all for your responses!

brendt hess Hall of Fame Points: 3256 More actions · Answer 9

Well, I just saw this, and there is a way. Try:

CREATE PROCEDURE dbo.spRef_GetCertifications

(

@activesOnly bit

)

AS

SELECT ID, Name, IsActive, SubmittedOnGMT FROM Certification WHERE IsActive >= @activesOnly ORDER BY Name ASC

lenardd Default port Points: 1435 More actions · Answer 10

brendthess,

Damn, that is a SLICK but ELEGANT way of doing it. Thanks! I must admit, at first glance, I had to strain my brain "a little" to figure what you were doing. I would have never thought of describing it that way.

Good Job!

--Lenard

5409045121009-7368 SSCoach Points: 15740 More actions · Answer 11

5409045121009-7368

SSCoach

Points: 15740

July 22, 2003 at 6:06 pm

#466497

Lifting the bar on '@activesOnly'

lenardd Default port Points: 1435 More actions · Answer 12

lenardd

Default port

Points: 1435

July 22, 2003 at 6:59 pm

#466501

Yup.....almost as good as Einstein's MC^2.

Frank Kalis SSC Guru Points: 111183 More actions · Answer 13

Hi brendthess

quote:
Well, I just saw this, and there is a way. Try:
CREATE PROCEDURE dbo.spRef_GetCertifications
(
@activesOnly bit
)
AS
SELECT ID, Name, IsActive, SubmittedOnGMT FROM Certification WHERE IsActive >= @activesOnly ORDER BY Name ASC

wummtralala, cool stuff!

Cheers,

Frank

--
Frank Kalis
Microsoft SQL Server MVP
Webmaster: http://www.insidesql.org/blogs
My blog: http://www.insidesql.org/blogs/frankkalis/[/url]

David Burrows SSC Guru Points: 65142 More actions · Answer 14

Excellent solution Brendt, wish I'd thought of that

Far away is close at hand in the images of elsewhere.
Anon.