2
\$\begingroup\$

Context

I was proud of this code for a little while, but the repetition wounds me. I know that the function name sucks, but the name and interface to this function are not under my control. Performance is not a concern.

Fixing the repetition is my only concern. How can I reduce it?

Note the usage of the tag. If you suggest a multi-statement table-valued function, then you've failed me. This must be an inline table-valued function.

Dependencies

CREATE TABLE [WINNERS]
(
  WINNER_ID INT NOT NULL
);

CREATE TABLE [RelevantWinners]
(
  WINNER_ID INT NOT NULL
);

INSERT INTO [WINNERS]
  VALUES (1), (2), (3), (4), (5);

INSERT INTO [RelevantWinners]
  VALUES (1), (5);

The Code For Review

CREATE FUNCTION RecentWinners
(
  @GetAllWinners BIT
)
RETURNS TABLE
AS
RETURN
(
-- Style note: I've yet to figure out how to best indent consecutive CTEs.
WITH
  [WinnersRelevant] AS
  (
    SELECT
      -- The inclusion of a silly calculation is deliberate.  
      2 * [Win].[WINNER_ID] + 47 [ENCODING]
    FROM
      [Winners] [Win]
    WHERE EXISTS
      (
          SELECT 1
          FROM [RelevantWinners] [Rev]
          WHERE [Rev].[WINNER_ID] = [Win].[WINNER_ID]
      )
  ),
  [WinnersAll] AS
  (
      -- This is the first major repetition.
      -- It's exactly the same code as before, but without the semi-join.
    SELECT
      2 * [Win].[WINNER_ID] + 47 [ENCODING]
    FROM
      [Winners] [Win]
  )
    SELECT
      [ENCODING],
      -- The inclusion of a second silly calculation is deliberate.
      -- Pretend it's independent of the previous one. 
      2 * [ENCODING] + 12346 [ENCODING_2]
    FROM
      [WinnersRelevant]
    WHERE
      @GetAllWinners = 0
 UNION ALL
      -- This is the second major repetition.
      -- It's exactly the same SELECT list as before.
    SELECT
      [ENCODING],
      2 * [ENCODING] + 12346 [ENCODING_2]
    FROM 
      [WinnersAll]
    WHERE
      @GetAllWinners = 1
); --I'm pretty sure this is the only relevant semicolon, even under strict standards.

DB Fiddle.

\$\endgroup\$
4
  • \$\begingroup\$ Yes, that's much improved. Thank you. \$\endgroup\$ Sep 29, 2023 at 17:15
  • 4
    \$\begingroup\$ BTW, although you say you have only one concern, be aware that here on CR, reviewers may critique any and all aspects of your code, regardless. Don't be offended if your focus is ignored. \$\endgroup\$ Sep 29, 2023 at 17:17
  • 1
    \$\begingroup\$ It's somewhat a stretch to call a single-query stored procedure functional programming. \$\endgroup\$
    – Reinderien
    Oct 1, 2023 at 12:45
  • \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. What you can do is ask a follow up question with a link back to this question. \$\endgroup\$
    – pacmaninbw
    Oct 1, 2023 at 13:08

2 Answers 2

2
\$\begingroup\$

Supply the function with

SELECT 0 AS idx, *   FROM Winners
UNION ALL
SELECT 1 AS idx, *   FROM RelevantWinners

and you'll have enough information available for a single SELECT to do what's needed.

\$\endgroup\$
3
  • \$\begingroup\$ That might do it. For some reason, I'm finding it very hard to picture. I'll give this another look tomorrow. Maybe it's been a long day and I'm just being stupid. \$\endgroup\$
    – J. Mini
    Sep 29, 2023 at 17:20
  • \$\begingroup\$ Thank you. This answer was so clever that it made me realise that I over-simplified my problem statement. I've now edited the question. \$\endgroup\$
    – J. Mini
    Oct 1, 2023 at 8:40
  • \$\begingroup\$ Excellent. Happy to help. I invite you to answer your own original (or more detailed) question. \$\endgroup\$
    – J_H
    Oct 1, 2023 at 15:27
1
\$\begingroup\$

Hullo, @J. Mini. I am deliberately responding to the followup over here, to keep the followup clean. We can move it later if warranted.

Oh, and thank you for the fiddle!


I find the original query hard to read, because I do not understand what it means at a business level. And each intentionally simplified "silly expression", and vague identifiers like encoding_2 don't really help me out.

But let me offer an intermediate result that perhaps gets you a bit closer to what you're looking for.

I'm assuming that each silly expression is "long" in its real form, and should appear Exactly Once, no copy-pasta allowed. Therefore I wound up with cascaded VIEWs, since SQL doesn't support SELECT foo * bar + baz AS complex, qux * complex.


CREATE VIEW both AS
  SELECT  0 AS idx, rw.winner_id, w.amount  FROM RelevantWinners rw  LEFT OUTER JOIN winners w  ON rw.winner_id = w.winner_id
  UNION ALL
  SELECT  1 AS idx, *                       FROM winners
;
CREATE VIEW answer1 AS
  SELECT  *, 2 * amount + 47  AS encoding
  FROM both
;
CREATE VIEW answer2 AS
  SELECT  *, encoding * 12346 AS encoding_2
  FROM answer1
;

The both relation looks like this.

| idx | winner_id | amount |
|-----+-----------+--------|
|   0 |         1 |     20 |
|   0 |         5 |     50 |
|   1 |         1 |     20 |
|   1 |         2 |     30 |
|   1 |         3 |    309 |
|   1 |         4 |     50 |
|   1 |         5 |     50 |

The answer2 relation is:

| idx | winner_id | amount | encoding | encoding_2 |
|-----+-----------+--------+----------+------------|
|   0 |         1 |     20 |       87 |    1074102 |
|   0 |         5 |     50 |      147 |    1814862 |
|   1 |         1 |     20 |       87 |    1074102 |
|   1 |         2 |     30 |      107 |    1321022 |
|   1 |         3 |    309 |      665 |    8210090 |
|   1 |         4 |     50 |      147 |    1814862 |
|   1 |         5 |     50 |      147 |    1814862 |

How close is that?

I suspect the piece I'm missing is we need some NULL amount rows? From a LEFT JOIN?


As Reinderien observes over in the followup, nested sub-queries will also fit the bill nicely.

\$\endgroup\$
2
  • \$\begingroup\$ This is a good effort, but I'm not really seeing the distinction between this and the original. All that I'm seeing is that it's converted the CTEs to views and omitted the parameter. \$\endgroup\$
    – J. Mini
    Oct 4, 2023 at 10:10
  • \$\begingroup\$ Just trying to put all that's needed into a single relation that we can report on. But yeah, I agree. I like Reinderien's answer much better, am giving it a +1. \$\endgroup\$
    – J_H
    Oct 4, 2023 at 17:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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