Jessica recently started at a company still using Windows Forms.
Well, that was a short article. Oh, you want more WTF than that? Sure, we can do that.
As you might imagine, a company that’s still using Windows Forms isn’t going to upgrade any time soon; they’ve been using an API that’s been in maintenance mode for a decade, clearly they’re happy with it.
But they’re not too happy- Jessica was asked to track down a badly performing report. This of course meant wading through a thicket of spaghetti code, pointless singletons, and the general sloppiness that is the code base. Some of the code was written using Entity Framework for database access, much of it is not.
While it wasn’t the report that Jessica was sent to debug, this method caught her eye:
<span class="hljs-function"><span class="hljs-keyword">private</span> Dictionary<<span class="hljs-built_in">long</span>, <span class="hljs-built_in">decimal</span>> <span class="hljs-title">GetReportDiscounts</span>(<span class="hljs-params">ReportCriteria criteria</span>)</span>
{
Dictionary<<span class="hljs-built_in">long</span>, <span class="hljs-built_in">decimal</span>> rows = <span class="hljs-keyword">new</span> Dictionary<<span class="hljs-built_in">long</span>, <span class="hljs-built_in">decimal</span>>();
<span class="hljs-built_in">string</span> query = <span class="hljs-string">@"select ii.IID,
SUM(CASE WHEN ii.AdjustedTotal IS NULL THEN
(ii.UnitPrice * ii.Units) ELSE
ii.AdjustedTotal END) as 'Costs'
from ii
where ItemType = 3
group by ii.IID
"</span>;
<span class="hljs-built_in">string</span> connectionString = <span class="hljs-built_in">string</span>.Empty;
<span class="hljs-keyword">using</span> (DataContext db = DataContextFactory.GetInstance<DataContext>())
{
connectionString = db.Database.Connection.ConnectionString;
}
<span class="hljs-keyword">using</span> (SqlConnection connection = <span class="hljs-keyword">new</span> SqlConnection(connectionString))
{
<span class="hljs-keyword">using</span> (SqlCommand command = <span class="hljs-keyword">new</span> SqlCommand(query, connection))
{
command.Parameters.AddWithValue(<span class="hljs-string">"@DateStart"</span>, criteria.Period.Value.Min.Value.Date);
command.Parameters.AddWithValue(<span class="hljs-string">"@DateEnd"</span>, criteria.Period.Value.Max.Value.Date.AddDays(<span class="hljs-number">1</span>));
command.Connection.Open();
<span class="hljs-keyword">using</span> (SqlDataReader reader = command.ExecuteReader())
{
<span class="hljs-keyword">while</span> (reader.Read())
{
<span class="hljs-built_in">decimal</span> discount = (<span class="hljs-built_in">decimal</span>)reader[<span class="hljs-string">"Costs"</span>];
<span class="hljs-built_in">long</span> IID = (<span class="hljs-built_in">long</span>)reader[<span class="hljs-string">"IID"</span>];
<span class="hljs-keyword">if</span> (rows.ContainsKey(IID))
{
rows[IID] += discount;
}
<span class="hljs-keyword">else</span>
{
rows.Add(IID, discount);
}
}
}
}
}
<span class="hljs-keyword">return</span> rows;
}
This code constructs a query, opens a connection, runs the query, and iterates across the results, building a dictionary as its result set. The first thing which leaps out is that, in code, they’re doing a summary (iterating across the results and grouping by IID), which is also what they did in the query.
It’s also notable that the table they’re querying is called ii, which is not a result of anonymization, and actually what they called it. Then there’s the fact that they set parameters on the query, for DateStart and DateEnd, but the query doesn’t use those. And then there’s that magic number 3 in the query, which is its own set of questions.
Then, right beneath that method was one called GetReportTotals. I won’t share it, because it’s identical to what’s above, with one difference:
<span class="hljs-built_in">string</span> query = <span class="hljs-string">@"
select ii.IID,
SUM(CASE WHEN ii.AdjustedTotal IS NULL THEN
(ii.UnitPrice * ii.Units) ELSE
ii.AdjustedTotal END) as 'Costs' from ii
where itemtype = 0
group by iid
"</span>;
The magic number is now zero.
So, clearly we’re in the world of copy/paste programming, but this raises the question: which came first, the 0 or the 3? The answer is neither. GetCancelledInvoices came first.
<span class="hljs-function"><span class="hljs-keyword">private</span> List<ReportDataRow> <span class="hljs-title">GetCancelledInvoices</span>(<span class="hljs-params">ReportCriteria criteria, Dictionary<<span class="hljs-built_in">long</span>, <span class="hljs-built_in">string</span>> dictOfInfo</span>)</span>
{
List<ReportDataRow> rows = <span class="hljs-keyword">new</span> List<ReportDataRow>();
<span class="hljs-built_in">string</span> fCriteriaName = <span class="hljs-string">"All"</span>;
<span class="hljs-built_in">string</span> query = <span class="hljs-string">@"select
A long query that could easily be done in EF, or at worst a stored procedure or view. Does actually use the associated parameters"</span>;
<span class="hljs-built_in">string</span> connectionString = <span class="hljs-built_in">string</span>.Empty;
<span class="hljs-keyword">using</span> (DataContext db = DataContextFactory.GetInstance<DataContext>())
{
connectionString = db.Database.Connection.ConnectionString;
}
<span class="hljs-keyword">using</span> (SqlConnection connection = <span class="hljs-keyword">new</span> SqlConnection(connectionString))
{
<span class="hljs-keyword">using</span> (SqlCommand command = <span class="hljs-keyword">new</span> SqlCommand(query, connection))
{
command.Parameters.AddWithValue(<span class="hljs-string">"@DateStart"</span>, criteria.Period.Value.Min.Value.Date);
command.Parameters.AddWithValue(<span class="hljs-string">"@DateEnd"</span>, criteria.Period.Value.Max.Value.Date.AddDays(<span class="hljs-number">1</span>));
command.Connection.Open();
<span class="hljs-keyword">using</span> (SqlDataReader reader = command.ExecuteReader())
{
<span class="hljs-keyword">while</span> (reader.Read())
{
<span class="hljs-built_in">long</span> ID = (<span class="hljs-built_in">long</span>)reader[<span class="hljs-string">"ID"</span>];
<span class="hljs-built_in">decimal</span> costs = (<span class="hljs-built_in">decimal</span>)reader[<span class="hljs-string">"Costs"</span>];
<span class="hljs-built_in">string</span> mNumber = (<span class="hljs-built_in">string</span>)reader[<span class="hljs-string">"MNumber"</span>];
<span class="hljs-built_in">string</span> mName = (<span class="hljs-built_in">string</span>)reader[<span class="hljs-string">"MName"</span>];
DateTime idate = (DateTime)reader[<span class="hljs-string">"IDate"</span>];
DateTime lastUpdatedOn = (DateTime)reader[<span class="hljs-string">"LastUpdatedOn"</span>];
<span class="hljs-built_in">string</span> iNumber = reader[<span class="hljs-string">"INumber"</span>] <span class="hljs-keyword">is</span> DBNull ? <span class="hljs-built_in">string</span>.Empty : (<span class="hljs-built_in">string</span>)reader[<span class="hljs-string">"INumber"</span>];
<span class="hljs-built_in">long</span> fId = (<span class="hljs-built_in">long</span>)reader[<span class="hljs-string">"FID"</span>];
<span class="hljs-built_in">string</span> empName = (<span class="hljs-built_in">string</span>)reader[<span class="hljs-string">"EmpName"</span>];
<span class="hljs-built_in">string</span> empNumber = reader[<span class="hljs-string">"EmpNumber"</span>] <span class="hljs-keyword">is</span> DBNull ? <span class="hljs-built_in">string</span>.Empty : (<span class="hljs-built_in">string</span>)reader[<span class="hljs-string">"empNumber"</span>];
<span class="hljs-built_in">long</span> mId = (<span class="hljs-built_in">long</span>)reader[<span class="hljs-string">"MID"</span>];
<span class="hljs-built_in">string</span> cName = dictOfInfo[matterId];
<span class="hljs-keyword">if</span> (criteria.EmployeeID.HasValue && fId != criteria.EmployeeID.Value)
{
<span class="hljs-keyword">continue</span>;
}
rows.Add(<span class="hljs-keyword">new</span> ReportDataRow()
{
CName = cName,
IID = ID,
Costs = costs * <span class="hljs-number">-1</span>, <span class="hljs-comment">//Cancelled i - minus PC</span>
TimedValue = <span class="hljs-number">0</span>,
MNumber = mNumber,
MName = mName,
BillDate = lastUpdatedOn,
BillNumber = iNumber + <span class="hljs-string">"A"</span>,
FID = fId,
EmployeeName = empName,
EmployeeNumber = empNumber
});
}
}
}
}
<span class="hljs-keyword">return</span> rows;
}
This is the original version of the method. We can infer this because it actually uses the parameters of DateStart and DateEnd. Everything else just copy/pasted this method and stripped out bits until it worked. There are more children of this method, each an ugly baby of its own, but all alike in their ugliness.
It’s also worth noting, the original version is doing filtering after getting data from the database, instead of putting that criteria in the WHERE clause.
As for Jessica’s poor performing report, it wasn’t one of these methods. It was, however, another variation on “run a query, then filter, sort, and summarize in C#”. By simply rewriting it as a SQL query in a stored procedure that leveraged indexes, performance improved significantly.
[Advertisement]Source: Read MoreÂ
