2
\$\begingroup\$

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
 )
 )
);
asked Aug 5, 2020 at 6:39
\$\endgroup\$
5
  • 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\$ Commented Aug 5, 2020 at 6:49
  • 1
    \$\begingroup\$ Also please tag the question with appropriate SQL dialect. Is this MS SQL Server? \$\endgroup\$ Commented 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\$ Commented 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\$ Commented 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\$ Commented Aug 6, 2020 at 9:22

1 Answer 1

1
\$\begingroup\$

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]
 )
 )
 );
answered Nov 6, 2020 at 3:36
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.