From 18b27b73c4a2ad0aecf9d7324786a13b0c88b913 Mon Sep 17 00:00:00 2001
From: Thom Lamb <tlamb@crateandbarrel.com>
Date: Fri, 29 May 2026 14:37:55 -0500
Subject: [PATCH] chore: refactoring the remaining code smells

---
 .../Breakdowns/QueryBreakdown.cs              | 80 +++++++------------
 .../Statements/ParsePreparation.cs            | 10 ++-
 2 files changed, 33 insertions(+), 57 deletions(-)

diff --git a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs b/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs
index 2382e06..159adce 100644
--- a/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs
+++ b/src/Strata.SqlTools.SqlServer/Breakdowns/QueryBreakdown.cs
@@ -1,5 +1,3 @@
-using System.Collections;
-using System.Text;
 using Strata.SqlTools.SqlBreakdown.Classes;
 using Strata.SqlTools.SqlBreakdown.Expressions;
 using Strata.SqlTools.SqlBreakdown.Interfaces;
@@ -7,6 +5,8 @@ using Strata.SqlTools.SqlBreakdown.Interfaces.QueryEngine;
 using Strata.SqlTools.SqlServer.Exceptions;
 using Strata.SqlTools.Statements.SqlServer;
 using Strata.SqlTools.Visitors.SqlServer;
+using System.Collections;
+using System.Text;
 
 namespace Strata.SqlTools.Breakdowns.SqlServer;
 
@@ -621,66 +621,40 @@ public class QueryBreakdown : SqlBreakdownBase, IQueryBreakdown
 
         AppendWithClauseSection(sb, withClauseIndent: "     ", queryBodyIndent: "          ");
 
-        sb.AppendLine("SELECT ");
-        if (!string.IsNullOrEmpty(SelectClause.Comment))
-        {
-            sb.AppendLine($"     {SelectClause.Comment}");
-        }
-        sb.AppendLine($"     {SelectClause.Clause}");
+        AppendClauseSection(sb, "SELECT", SelectClause);
 
-        if (IsUsingFromClause)
-        {
-            sb.AppendLine("FROM ");
-            if (!string.IsNullOrEmpty(FromClause.Comment))
-            {
-                sb.AppendLine($"     {FromClause.Comment}");
-            }
-            sb.AppendLine($"     {FromClause.Clause}");
-        }
+        if (IsUsingFromClause) AppendClauseSection(sb, "FROM", FromClause);
 
-        if (IsUsingWhereClause)
-        {
-            sb.AppendLine("WHERE ");
-            if (!string.IsNullOrEmpty(WhereClause.Comment))
-            {
-                sb.AppendLine($"     {WhereClause.Comment}");
-            }
-            sb.AppendLine($"     {WhereClause.Clause}");
-        }
+        if (IsUsingWhereClause) AppendClauseSection(sb, "WHERE", WhereClause);
 
-        if (IsUsingGroupByClause)
-        {
-            sb.AppendLine("GROUP BY ");
-            if (!string.IsNullOrEmpty(GroupByClause.Comment))
-            {
-                sb.AppendLine($"     {GroupByClause.Comment}");
-            }
-            sb.AppendLine($"     {GroupByClause.Clause}");
-        }
+        if (IsUsingGroupByClause) AppendClauseSection(sb, "GROUP BY", GroupByClause);
 
-        if (IsUsingHavingClause)
-        {
-            sb.AppendLine("HAVING ");
-            if (!string.IsNullOrEmpty(HavingClause.Comment))
-            {
-                sb.AppendLine($"     {HavingClause.Comment}");
-            }
-            sb.AppendLine($"     {HavingClause.Clause}");
-        }
+        if (IsUsingHavingClause) AppendClauseSection(sb, "HAVING", HavingClause);
 
-        if (IsUsingOrderByClause)
-        {
-            sb.AppendLine("ORDER BY ");
-            if (!string.IsNullOrEmpty(OrderByClause.Comment))
-            {
-                sb.AppendLine($"     {OrderByClause.Comment}");
-            }
-            sb.AppendLine($"     {OrderByClause.Clause}");
-        }
+        if (IsUsingOrderByClause) AppendClauseSection(sb, "ORDER BY", OrderByClause);
 
         return sb.ToString();
     }
 
+    /// <summary>
+    /// Appends a single clause section — the keyword line, the optional comment line,
+    /// and the indented clause body — using the layout shared by every SELECT-query clause.
+    /// </summary>
+    /// <param name="sb">The builder accumulating the SQL.</param>
+    /// <param name="keyword">The leading SQL keyword (e.g. <c>"SELECT"</c>, <c>"GROUP BY"</c>).</param>
+    /// <param name="clause">The clause whose comment and body are rendered.</param>
+    private static void AppendClauseSection(StringBuilder sb, string keyword, ISqlClause clause)
+    {
+        if (string.IsNullOrEmpty(keyword)) return;
+
+        sb.AppendLine($"{keyword} ");
+        if (!string.IsNullOrEmpty(clause.Comment))
+        {
+            sb.AppendLine($"     {clause.Comment}");
+        }
+        sb.AppendLine($"     {clause.Clause}");
+    }
+
     /// <summary>
     /// Gets the complete SQL query including optional setup and finish clauses (for backward compatibility).
     /// </summary>
diff --git a/src/Strata.SqlTools.SqlServer/Statements/ParsePreparation.cs b/src/Strata.SqlTools.SqlServer/Statements/ParsePreparation.cs
index e3eadce..8cc6717 100644
--- a/src/Strata.SqlTools.SqlServer/Statements/ParsePreparation.cs
+++ b/src/Strata.SqlTools.SqlServer/Statements/ParsePreparation.cs
@@ -1,6 +1,7 @@
-using System.Collections;
-using System.Text.RegularExpressions;
 using Strata.SqlTools.SqlBreakdown.Utilities;
+using System.Collections;
+using System.Diagnostics.CodeAnalysis;
+using System.Text.RegularExpressions;
 
 namespace Strata.SqlTools.Statements.SqlServer;
 
@@ -29,6 +30,7 @@ public static class ParsePreparation
     /// <param name="finishClauses">On success, the finish clauses extracted from the back of the SQL.</param>
     /// <param name="errorMessage">On failure, a human-readable explanation suitable for surfacing through the caller's <c>out string errorMessage</c>.</param>
     /// <returns><c>true</c> if the prelude completed and <paramref name="normalizedSql"/> is ready for dialect-specific matching; <c>false</c> otherwise.</returns>
+    [SuppressMessage("Major Code Smell", "S107:Methods should not have too many parameters", Justification = "This helper consolidates the identical TryParse prelude shared across the dialect breakdown classes. The parameter count reflects the prelude's inputs (sql, parser, prefix regex/description) plus its four out results (normalizedSql, setupClauses, finishClauses, errorMessage) mirroring the caller's out-parameter contract; collapsing them into a result struct would obscure the drop-in TryParse signature it replaces.")]
     public static bool TryRunPrelude(
         string sql,
         StatementParser parser,
@@ -58,10 +60,10 @@ public static class ParsePreparation
             return false;
         }
 
-        setupClauses = new List<string>();
+        setupClauses = [];
         normalizedSql = parser.ExtractSetupClauses(normalizedSql, setupClauses);
 
-        finishClauses = new ArrayList();
+        finishClauses = [];
         normalizedSql = parser.ExtractFinishClauses(normalizedSql, finishClauses);
 
         return true;