This question is abut coding style. I'd like to see how would you improve this code look considering readability as well. Feel free to upvote others solutions you find good.
#1 Initial version:
SELECT greatest(
0,
least(
500,
(SELECT g.c_daily_limit * (
SELECT count(*)
FROM rivals.t_rival_group_auth a
WHERE a.id_group = g.id) - (
SELECT count(*)
FROM rivals.t_order_results rs
JOIN rivals.t_rivals r ON r.id = rs.id_rival
WHERE r.id_group = g.id
AND rs.c_date_order::date = utils.getdate()::date
AND (rs.c_price IS NOT NULL OR rs.c_failed IS NOT NULL))
FROM rivals.t_rival_group g
WHERE g.c_code = _code)
)
);
#2 An alternative version, should improve readability of nested parameters:
SELECT greatest(
0,
least(
500,
(
SELECT g.c_daily_limit * (
SELECT count(*)
FROM rivals.t_rival_group_auth a
WHERE a.id_group = g.id
) - (
SELECT count(*)
FROM rivals.t_order_results rs
JOIN rivals.t_rivals r ON r.id = rs.id_rival
WHERE r.id_group = g.id
AND rs.c_date_order::date = utils.getDate()::date
AND (rs.c_price IS NOT NULL OR rs.c_failed IS NOT NULL)
)
FROM rivals.t_rival_group g
WHERE g.c_code = _code
)
)
);
-
2\$\begingroup\$ Welcome to Code Review! It appears that you are trying to do a comparative-review. Please add the solutions contained in the answers to your question. \$\endgroup\$Sᴀᴍ Onᴇᴌᴀ– Sᴀᴍ Onᴇᴌᴀ ♦2020年08月05日 06:49:28 +00:00Commented Aug 5, 2020 at 6:49
-
1\$\begingroup\$ Also please tag the question with appropriate SQL dialect. Is this MS SQL Server? \$\endgroup\$slepic– slepic2020年08月05日 08:53:00 +00:00Commented Aug 5, 2020 at 8:53
-
2\$\begingroup\$ Can't test right now but I would look into making this a CTE. If you add table structure + some data and post on Dbfiddle for example, then it will be possible to test something. \$\endgroup\$Kate– Kate2020年08月05日 16:33:19 +00:00Commented Aug 5, 2020 at 16:33
-
2\$\begingroup\$ The current question title of your question is too generic to be helpful. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$BCdotWEB– BCdotWEB2020年08月06日 09:20:42 +00:00Commented Aug 6, 2020 at 9:20
-
2\$\begingroup\$ You should provide context for your query, i.e. what does it do. Also, you should post the code to create the relevant tables. Please follow the tour, and read "What topics can I ask about here?", "How do I ask a good question?" and "What types of questions should I avoid asking?". \$\endgroup\$BCdotWEB– BCdotWEB2020年08月06日 09:22:33 +00:00Commented Aug 6, 2020 at 9:22
1 Answer 1
The following are a few suggestions on how I'd write the SQL.
Source Control
If you don't already have a database project, create one in Visual Studio. Then check it in to source control. Microsoft Azure DevOps Services is free & private for teams of 5 or less (this is per project, so 5 developers per project). Then you'll be able to track changes you make to your stored procedures, views, tables, etc.
Formatting
I use the following tools for SSMS and Visual Studio, ApexSQL Refactor and Poor Man's T-Sql Formatter. I use it when I have to edit other developer's code. It's a great way to standardize your SQL. I find it does most of the formatting for me, but I'll still make a few changes after.
Copy & Paste
If you find yourself copying and pasting the same string or number over and over in your query, then you should define it as a variable. Copy and paste is a design error ~ David Parnas
Commas
I would put the commas in front to clearly define new columns. Versus code wrapped in multiple lines. It also makes trouble-shooting code easier.
Where Clause
If you put 1=1
at the top of a WHERE
condition, it enables you to freely change the rest of the conditions when debugging a query. The SQL query engine will end up ignoring the 1=1
so it should have no performance impact. Reference
Common Table Expressions (CTE)
CTE's in your SQL help with documentation. The expression name can then let other developers know why you used that expression e.g. number_of_group_auth
or number_of_order_results
.
Schema Names
Always reference the schema when selecting an object e.g. [dbo].[Sales]
.
- Also check out the book Clean Code. It will change the way you think about naming conventions.
Revised SQL
Without table definitions and sample records I was unable to test this, but it should give you a good start.
WITH
number_of_group_auth -- table expression names should tell you what they're doing so you don't need to use comments
AS
(
SELECT
[group_count] = COUNT(*)
, [a].[id_group]
FROM
[rivals].[t_rival_group_auth] AS [a]
)
,
number_of_order_results
AS
(
SELECT
[group_count] = COUNT(*)
, [r].[id_group]
FROM
[rivals].[t_order_results] AS [rs]
INNER JOIN [rivals].[t_rivals] AS [r] ON [r].[id] = [rs].[id_rival]
WHERE
1=1
AND [rs].[c_date_order]::date = utils.getdate() ::date
AND
(
[rs].[c_price] IS NOT NULL
OR [rs].[c_failed] IS NOT NULL
)
)
SELECT
greatest
(0, least
(500,
(
SELECT
[g].[c_daily_limit] * [number_of_group_auth].[group_count] - [number_of_order_results].[group_count]
FROM
[rivals].[t_rival_group] AS [g]
LEFT JOIN [number_of_group_auth] AS [a] ON [a].[id_group] = [g].[id]
LEFT JOIN [number_of_order_results] AS [r] ON [r].[id_group] = [g].[id]
WHERE
[g].[c_code] = [_code]
)
)
);