Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

So I got this query from another analyst here and I'm a little stumped as to why a cursor was used instead of just joins. Could I please have some help trying to refactor it? I am using MSSQL 2008.

SET NOCOUNT ON;

DECLARE @myCur AS CURSOR;
DECLARE @totalCdes AS INT;
DECLARE @SysOjb AS SMALLINT;
DECLARE @prinOjb AS SMALLINT;
DECLARE @JobClass AS VARCHAR (25);
DECLARE @JobNo AS INT;
DECLARE @JobType AS VARCHAR (2);
DECLARE @JobDescr AS VARCHAR (30);
DECLARE @JobComplCde AS VARCHAR (18);
DECLARE @OrderNo AS VARCHAR (16);
DECLARE @SchedDate AS DATETIME;
DECLARE @JobResolution AS VARCHAR (200);
DECLARE @startingRow AS INT;

CREATE TABLE #JobsFixCodes
(
    SYS_OJB        BIGINT       ,
    PRIN_OJB       SMALLINT     ,
    JOB_NO         INT          ,
    ORDER_NO       BIGINT       ,
    JOB_CLASS      VARCHAR (25) ,
    JOB_TYPE       VARCHAR (30) ,
    JOB_RESOLUTION VARCHAR (200),
    SCHED_DATE     DATE         
);

SET @myCur = CURSOR
    FOR SELECT [SYS_OJB],
            [PRIN_OJB],
            CASE [JOB_CLASS_OJB] 
WHEN 'C' THEN 'New Connect' 
WHEN 'D' THEN 'Disco' 
WHEN 'R' THEN 'Restart' 
WHEN 'S' THEN 'Service Change' 
WHEN 'T' THEN 'Trouble Call' 
WHEN 'Z' THEN 'Special Request' ELSE 'Unknown' 
END AS JobClass,
            [JOB_NO_OJB],
            [JOB_TYP_OJB],
            COALESCE (cagent.DESCR_CTD, cprin.DESCR_CTD, csys.DESCR_CTD, 'Unknown') AS Job_Descr,
            COMPL_CDE_OJB,
            [ORDER_NO_OJB],
            [SCHED_DTE_OJB]
        FROM   [ExternalUser].[Vantage].[OJB_JOBS] AS j
            LEFT OUTER JOIN
            [ExternalUser].[Vantage].[CTD_DISPLAY] AS cagent
            ON cagent.SYS_CTD = j.SYS_OJB
                AND cagent.PRIN_CTD = j.PRIN_OJB
                AND cagent.AGNT_CTD = j.AGNT_OJB
                AND cagent.CDE_TBL_NO_CTD = '32'
                AND cagent.CDE_VALUE_CTD = j.JOB_TYP_OJB
                AND cagent.SPA_FLG_CTD = 'A'
            LEFT OUTER JOIN
            [ExternalUser].[Vantage].[CTD_DISPLAY] AS cprin
            ON cprin.SYS_CTD = j.SYS_OJB
                AND cprin.PRIN_CTD = j.PRIN_OJB
                AND cprin.CDE_TBL_NO_CTD = '32'
                AND cprin.CDE_VALUE_CTD = j.JOB_TYP_OJB
                AND cprin.SPA_FLG_CTD = 'P'
            LEFT OUTER JOIN
            [ExternalUser].[Vantage].[CTD_DISPLAY] AS csys
            ON csys.SYS_CTD = j.SYS_OJB
                AND csys.CDE_TBL_NO_CTD = '32'
                AND csys.CDE_VALUE_CTD = j.JOB_TYP_OJB
                AND csys.SPA_FLG_CTD = 'S'
        WHERE  JOB_STAT_OJB = 'C'
            AND SYS_OJB = '8155'
            --and SCHED_DTE_OJB = @DateParm
            AND SCHED_DTE_OJB = CONVERT (VARCHAR (20), GETDATE()-3, 101); /*Use this to run 3 days in arrears*/
            --and JOB_CLASS_OJB in (@JobClassParm) **Keep this commented out if you want *all* job classes
OPEN @myCur;

FETCH NEXT FROM @myCur INTO @SysOjb, @prinOjb, @JobClass, @JobNo, @JobType, @JobDescr, @JobComplCde, @OrderNo, @SchedDate;

WHILE @@FETCH_STATUS = 0
    BEGIN
        SET @totalCdes = len(@JobComplCde);
        --if @totalCdes is = 0 then don't loop
        IF @totalCdes > 0
            BEGIN
                IF @totalCdes = 3
                OR @totalCdes = 6
                OR @totalCdes = 9
                OR @totalCdes = 12
                OR @totalCdes = 15
                OR @totalCdes = 18
                    SET @totalCdes = @totalCdes / 3; --this is the true loop count
                ELSE
                    SET @totalCdes = @totalCdes / 2;
            END
        ELSE
            GOTO QuitMe;
        -- print 'Total job resolutions for order (: ' + convert(varchar(16), @OrderNo) + '): ' + convert(varchar(10), @totalCdes)
        IF @JobClass = 'Service Change'
        OR @JobClass = 'Special Request'
            WHILE @totalCdes > 0
                BEGIN
                    SET @startingRow = (@totalCdes * 3) - 2;
                    -- print 'Service Change in prin ' + convert(varchar(4),@prinOjb) + ' found with order: ' + convert(varchar(16), @OrderNo) + ' with ' + convert(varchar(16), @totalCdes) + ' resolutions of ' + @JobComplCde
                    -- print 'starting row is ' + convert(varchar(4),@startingRow)
                    SET @JobResolution = (SELECT descr_ctd
                                        FROM   [ExternalUser].[Vantage].[CTD_DISPLAY]
                                        WHERE  CDE_TBL_NO_CTD = '19'
                                                AND CDE_VALUE_CTD = SUBSTRING(@JobComplCde, @startingRow, 3)
                                                AND SYS_CTD = '8155');
                    INSERT  INTO #JobsFixCodes
                    VALUES (@SysOjb, @prinOjb, @JobNo, @OrderNo, @JobClass, @JobDescr, @JobResolution, @SchedDate);
                    SET @totalCdes = @totalCdes - 1;
                END
        IF @JobClass = 'Trouble Call'
            WHILE @totalCdes > 0
                BEGIN
                    SET @startingRow = (@totalCdes * 3) - 2;
                    -- print 'Service Change in prin ' + convert(varchar(4),@prinOjb) + ' found with order: ' + convert(varchar(16), @OrderNo) + ' with ' + convert(varchar(16), @totalCdes) + ' resolutions of ' + @JobComplCde
                    -- print 'starting row is ' + convert(varchar(4),@startingRow)
                    SET @JobResolution = (SELECT descr_ctd
                                        FROM   [ExternalUser].[Vantage].[CTD_DISPLAY]
                                        WHERE  CDE_TBL_NO_CTD = '08'
                                                AND CDE_VALUE_CTD = SUBSTRING(@JobComplCde, @startingRow, 3)
                                                AND SYS_CTD = '8155');
                    INSERT  INTO #JobsFixCodes
                    VALUES (@SysOjb, @prinOjb, @JobNo, @OrderNo, @JobClass, @JobDescr, @JobResolution, @SchedDate);
                    SET @totalCdes = @totalCdes - 1;
                END
        QuitMe:
        FETCH NEXT FROM @myCur INTO @SysOjb, @prinOjb, @JobClass, @JobNo, @JobType, @JobDescr, @JobComplCde, @OrderNo, @SchedDate;
    END

CLOSE @myCur;

DEALLOCATE @myCur;

SELECT   *
FROM     #JobsFixCodes AS f
ORDER BY f.JOB_CLASS, f.ORDER_NO, f.SCHED_DATE;

DROP TABLE #JobsFixCodes;
share|improve this question
Check out Jeff Atwood's post on Rubber Duck problem solving: codinghorror.com/blog/2012/03/rubber-duck-problem-solving.html – Tom Halladay May 8 '12 at 22:35
Refactoring complex Cursor procedures into proper SQL queries is Hard. That's why so many people here would rather not have to think about it. Take a look at this series of article (There Must Be 50 Ways to Lose Your Cursors), written explicitly to help developers refactor thier Cursors into good SQL queries. Sadly, the notoriously opinionated author never finished the series, but these first two article should be perfect for helping you to get started with the refactoring process. – RBarryYoung May 9 '12 at 1:27
So, the question was migrated here, after and answer was already posted, and then my answer was turned into a comment and I was never notified? Nice. Sadly, this is the type of classless, high-handed behavior I have come to expect from StackExchange. – RBarryYoung May 9 '12 at 20:04

migrated from stackoverflow.com May 9 '12 at 11:51

1 Answer

Refactoring complex Cursor procedures into proper SQL queries is Hard. That's why so many people here would rather not have to think about it. Take a look at this series of article (There Must Be 15 Ways to Lose Your Cursors), written explicitly to help developers refactor thier Cursors into good SQL queries. Sadly, the notoriously opinionated author never finished the series, but these first two article should be perfect for helping you to get started with the refactoring process.

share|improve this answer

Your Answer

 
discard

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