3
\$\begingroup\$

I've refactored my previous query and added better checks when selecting the data. The query link can be found here. The formulas used to calculate the two percentages are the same, and can be seen below:

$$ \text{Answered Percentage} = 100 - \left(\frac{\text{Unanswered}}{\text{Answered}} \times 100\right) \\ \text{Unanswered Percentage} = \frac{\text{Unanswered}}{\text{Answered}} \times 100 $$

Finally, here's the source.

DECLARE @TotalQuestions FLOAT = (
  SELECT COUNT(*) FROM Posts WHERE 
        PostTypeId = 1
    AND ClosedDate IS NULL
);

DECLARE @UnansweredQuestions FLOAT = (
  SELECT COUNT(*) 
    FROM Posts AS TopPostsQuery WHERE (
            ClosedDate IS NULL
        AND PostTypeId = 1
        AND AnswerCount = 0
      )
      OR (
            ClosedDate IS NULL
        AND PostTypeId = 1
        AND AnswerCount <= 1
        AND AcceptedAnswerId IS NULL
        AND (
          SELECT Score FROM Posts AS SubPostsQuery WHERE
                PostTypeId <> 1
            AND ParentId = TopPostsQuery.Id
        ) <= 0
      )
);

DECLARE @AnsweredPercentage FLOAT =
  100.0 - ((@UnansweredQuestions / @TotalQuestions) * 100.0);

DECLARE @UnansweredPercentage FLOAT = 
  (@UnansweredQuestions / @TotalQuestions) * 100.0;

SELECT
    [Percentage Of Answered Questions]   = @AnsweredPercentage
  , [Percentage Of Unanswered Questions] = @UnansweredPercentage;
\$\endgroup\$
1
\$\begingroup\$

Quite good!

I'll have to say it's well-written and clear to understand. Well done. There are a few things still you could improve, albeit they are somewhat minor.


Magic numbers

You use things like PostTypeId = 1 quite a bit. To an outsider, that doesn't mean much, other than that they'll have to query the PostTypes table to find out what it is. It also has the disadvantage that, in the event that it changed, for some reason (unlikely here, but still something to consider), your query could become broken.

Instead, I'd suggest to make a reusable variable:

DECLARE @Question INT = 1;

For a start. But, to make it to where even you, as the person who writes the query, don't need to know or care what the PostId for a question is (which is how it should be for IDs, they don't "mean" anything), try something like this:

DECLARE @Question INT = (SELECT Id FROM PostTypes WHERE Name = 'Question');

That way it's clear and pretty much foolproof.


FLOAT

Since a lot of SQL has to do with statistics and reporting, often a DECIMAL(m, n) type is preferred for output, where m is the maximum number of total digits, and n is how many of those digits are after the decimal point.

In your case, perhaps casting the results to a DECIMAL (7,4) would work pretty good, or even (5,2) to make the data easier to grasp.


Here is the resulting query.

\$\endgroup\$
2
\$\begingroup\$

Your code has certainly improved from last time, however there's a few things that could be improved:

  • AND AnswerCount <= 1: that could be AND AnswerCount < 0.
  • You don't need to recreate the amount of unanswered questions twice:
DECLARE @AnsweredPercentage FLOAT =
  100.0 - ((@UnansweredQuestions / @TotalQuestions) * 100.0);

DECLARE @UnansweredPercentage FLOAT = 
  (@UnansweredQuestions / @TotalQuestions) * 100.0;

into:

DECLARE @UnansweredPercentage FLOAT = 
  (@UnansweredQuestions / @TotalQuestions) * 100.0;

DECLARE @AnsweredPercentage FLOAT =
  100.0 - @UnansweredPercentage;
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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