Computer
CodeSOD: Gridding My Teeth
Dan's co-workers like passing around TDWTF stories, mostly because seeing code worse than what they're writing makes them feel less bad about how often they end up hacking things together.
One day, a co-worker told Dan: "Hey, I think I found something for that website with the bad code stories!"
Dan's heart sank. He didn't really want to shame any of his co-workers. Fortunately, the source-control history put the blame squarely on someone who didn't work there any more, so he felt better about submitting it.
This is another ASP .Net page, and this one made heavy use of GridView elements. GridView controls applied the logic of UI controls to generating a table. They had a page which contained six of these controls, defined like this:
<asp:GridView ID="gvTaskMonth1" runat="server" CssClass="leadsGridView" AutoGenerateColumns="False" OnRowDataBound="gvTaskMonth1_RowDataBound"> ... </asp:GridView> <asp:GridView ID="gvTaskMonth2" runat="server" CssClass="leadsGridView" AutoGenerateColumns="False" OnRowDataBound="gvTaskMonth1_RowDataBound"> ... </asp:GridView> <asp:GridView ID="gvTaskMonth3" runat="server" CssClass="leadsGridView" AutoGenerateColumns="False" OnRowDataBound="gvTaskMonth1_RowDataBound"> ... </asp:GridView>The purpose of this screen was to display a roadmap of coming tasks, broken up by how many months in the future they were. The first thing that leaps out to me is that they all use the same event handler for binding data to the table, which isn't in-and-of-itself a problem, but the naming of it is certainly a recipe for confusion.
Now, to bind these controls to the data, there needed to be some code in the code-behind of this view which handled that. That's where the WTF lurks:
/// <summary> /// Create a roadmap for the selected client /// </summary> private void CreateRoadmap() { for (int i = 1; i < 7; i++) { switch (i) { case 1: if (gvTaskMonth1.Rows.Count > 0) { InsertTasks(gvTaskMonth1, DateTime.Parse(txtDatePeriod1.Text), "1"); } break; case 2: if (gvTaskMonth2.Rows.Count > 0) { InsertTasks(gvTaskMonth2, DateTime.Parse(txtDatePeriod2.Text), "2"); } break; case 3: if (gvTaskMonth3.Rows.Count > 0) { InsertTasks(gvTaskMonth3, DateTime.Parse(txtDatePeriod3.Text), "3"); } break; case 4: if (gvTaskMonth4.Rows.Count > 0) { InsertTasks(gvTaskMonth4, DateTime.Parse(txtDatePeriod4.Text), "4"); } break; case 5: if (gvTaskMonth5.Rows.Count > 0) { InsertTasks(gvTaskMonth5, DateTime.Parse(txtDatePeriod5.Text), "5"); } break; case 6: if (gvTaskMonth6.Rows.Count > 0) { InsertTasks(gvTaskMonth6, DateTime.Parse(txtDatePeriod6.Text), "6"); } break; } } }Ah, the good old fashioned loop-switch sequence anti-pattern. I understand the motivation: "I want to do the same thing for six different controls, so I should use a loop to not repeat myself," but then couldn't quite figure out how to do that, so they just repeated themselves, but inside of a loop.
The "fix" was to replace all of this with something more compact:
private void CreateRoadmap() { InsertTasks(gvTaskMonth1, DateTime.Parse(txtDatePeriod1.Text), "1"); InsertTasks(gvTaskMonth2, DateTime.Parse(txtDatePeriod2.Text), "2"); InsertTasks(gvTaskMonth3, DateTime.Parse(txtDatePeriod3.Text), "3"); InsertTasks(gvTaskMonth4, DateTime.Parse(txtDatePeriod4.Text), "4"); InsertTasks(gvTaskMonth5, DateTime.Parse(txtDatePeriod5.Text), "5"); InsertTasks(gvTaskMonth6, DateTime.Parse(txtDatePeriod6.Text), "6"); }That said, I'd recommend not trying to parse date times inside of a text box inside of this method, but that's just me. Bubbling up the inevitable FormatException that this will generate is going to be a giant nuisance. It's likely that they've got a validator somewhere, so it's probably fine- I just don't like it.
.comment { border: none; } [Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.Canva Now Requires Use of LLMs During Coding Interviews
Read more of this story at Slashdot.
Abandoned Subdomains from Major Institutions Hijacked for AI-Generated Spam
Read more of this story at Slashdot.
Large Language Models, Small Labor Market Effects
Read more of this story at Slashdot.
An Experimental New Dating Site Matches Singles Based on Their Browser Histories
Read more of this story at Slashdot.
Talen Energy and Amazon Sign Nuclear Power Deal To Fuel Data Centers
Read more of this story at Slashdot.
Apple Quietly Launches Container On GitHub To Bring Linux Development To macOS
Read more of this story at Slashdot.
23andMe Says 15% of Customers Asked To Delete Their Genetic Data Since Bankruptcy
Read more of this story at Slashdot.
Nintendo Switch 2 Is Fastest-Selling Game Console of All Time
Read more of this story at Slashdot.
Amazon Is About To Be Flooded With AI-Generated Video Ads
Read more of this story at Slashdot.
Hong Kong Bans Video Game Using National Security Laws
Read more of this story at Slashdot.
Scientists Built a Badminton-Playing Robot With AI-Powered Skills
Read more of this story at Slashdot.
Airlines Don't Want You to Know They Sold Your Flight Data to DHS
Read more of this story at Slashdot.
Wikipedia Pauses AI-Generated Summaries After Editor Backlash
Read more of this story at Slashdot.
HP's First Google Beam 3D Video System Costs $24,999, Plus Unknown License Fees
Read more of this story at Slashdot.
Major Telescope Hosts World's Largest Digital Camera
Read more of this story at Slashdot.
Disney, NBCU Sue AI Image Generator Midjourney Over Copyright Infringement
Read more of this story at Slashdot.
WhatsApp Moves To Support Apple Against UK Government's Data Access Demands
Read more of this story at Slashdot.
Apple Executives Defend AI Strategy
Read more of this story at Slashdot.
Pirate Site Visits Dip To 216 Billion a Year, But Manga Piracy Is Booming
Read more of this story at Slashdot.