current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I'm getting three inputs and based on those three inputs, I'm returning. Input items may vary, so I used CASE WHEN statements here.

Is this code correct? If so, how can I make it more optimized?

ALTER Procedure [dbo].[WP_GetAllItems] 
    @IsActive     bit,
    @OrderMode    bit,
    @OrderBy      varchar(75)
as
Begin 
    Select ItemPartNumber, ItemDescription, CreatedDate, InitialPrice from Items where IsActive = @IsActive order by
    CASE WHEN @OrderBy='ItemDescription' AND @OrderMode = 0 THEN ItemDescription END ASC,
    CASE WHEN @OrderBy='ItemDescription' AND @OrderMode = 1 THEN ItemDescription END DESC,
    CASE WHEN @OrderBy='ItemPartNumber' AND @OrderMode = 0 THEN ItemPartNumber END ASC,
    CASE WHEN @OrderBy='ItemPartNumber' AND @OrderMode = 1 THEN ItemPartNumber END DESC,
    CASE WHEN @OrderBy='CreatedDate' AND @OrderMode = 0 THEN CreatedDate END ASC,
    CASE WHEN @OrderBy='CreatedDate' AND @OrderMode = 1 THEN CreatedDate END DESC,
    CASE WHEN @OrderBy='InitialPrice' AND @OrderMode = 0 THEN InitialPrice END ASC,
    CASE WHEN @OrderBy='InitialPrice' AND @OrderMode = 1 THEN InitialPrice END DESC
End
share|improve this question

1 Answer 1

up vote 1 down vote

It looks like your code produces the intended results (I assume you've tested it), but not for the right reasons.

You misunderstood the way CASE .. WHEN .. THEN .. END is intended to be used. Check out the official documentation. It has examples. There are supposed to be multiple WHEN .. THEN .. between the CASE and the END.

However, since there is just a single WHEN clause in each CASE clauses, if the WHEN clause is not satisfied, then the whole CASE .. END clause evaluates to NULL. ORDER BY NULL has no effect, so it turns out that only the clauses you want to use are being used. Lucky you.

However, it would improve the code to use CASE in the way it was intended. I can't do it all at once but I can refactor the code slightly.

ALTER Procedure [dbo].[WP_GetAllItems] 
    @IsActive        bit,
    @OrderMode       bit,
    @OrderBy         varchar(75)
as
Begin 
    Select ItemPartNumber, ItemDescription, CreatedDate, InitialPrice from Items where IsActive = @IsActive order by
    IIF( @OrderMode=0,
        CASE
            WHEN @OrderBy='ItemDescription' THEN ItemDescription
            WHEN @OrderBy='ItemPartNumber' THEN ItemPartNumber
            WHEN @OrderBy='CreatedDate' THEN CreatedDate
            WHEN @OrderBy='InitialPrice' THEN InitialPrice
        END, NULL ) ASC,
    IIF( @OrderMode=1,
        CASE
            WHEN @OrderBy='ItemDescription' THEN ItemDescription
            WHEN @OrderBy='ItemPartNumber' THEN ItemPartNumber
            WHEN @OrderBy='CreatedDate' THEN CreatedDate
            WHEN @OrderBy='InitialPrice' THEN InitialPrice
        END, NULL ) DESC
End
share|improve this answer
1  
Order by not working in this query. –  Programming Newbie May 5 at 4:53
    
thanks, I missed that when editing. –  Snowbody May 5 at 5:12

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.