Inability to properly program dates continued to afflict various websites last week, even though the leap day itself had passed. Maybe we need a new programming language in which it's impossible to forget about timezones, leap years, or Thursday.

Timeless Thomas subtweeted "I'm sure there's a great date-related WTF story behind this tweet" Gosh, I can't imagine what error this could be referring to.

 

Data historian Jonathan babbled "Today, the 1st of March, is the start of a new tax year here and my brother wanted to pull the last years worth of transactions from a financial institution to declare his taxes. Of course the real WTF is that they only allow up to 12 months." I am not able rightly to apprehend the confusion of ideas that could provoke such an error'd.

 

Ancient Matthew S. breathed a big sigh of relief on seeing this result: "Good to know that I'm up to date as of 422 years ago!"

 

Jaroslaw gibed "Looks like a translation mishap... What if I didn't knew English?" Indeed.

 

Hardjay vexillologist Julien casts some shade without telling us where to direct our disdain "I don't think you can have dark mode country flags..." He's not wrong.

 

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

Today, John sends us a confession. This is his code, which was built to handle ISO 8583 messages. As we'll see from some later comments, John knows this is bad.

The ISO 8583 format is used mostly in financial transaction processing, frequently to talk to ATMs, but is likely to show up somewhere in any transaction you do that isn't pure cash.

One of the things the format can support is bitmaps- not the image format, but the "stuff flags into an integer" format. John wrote his own version of this, in C#. It's a long class, so I'm just going to focus on the highlights.

private readonly bool[] bits;

Look, we don't start great. This isn't an absolute mistake, but if you're working on a data structure that is meant to be manipulated via bitwise operations, just lean into it. And yes, if endianness is an issue, you'll need to think a little harder- but you need to think about that anyway. Use clear method names and documentation to make it readable.

In this developer's defense, the bitmap's max size is 128 bits, which doesn't have a native integral type in C#, but a pair of 64-bits would be easier to understand, at least for me. Maybe I've just been infected by low-level programming brainworms. Fine, we're using an array.

Now, one thing that's important, is that we're using this bitmap to represent multiple things.

public bool IsExtendedBitmap { get { return this.IsFieldSet(1); } }

Note how the 1st bit in this bitmap is the IsExtendedBitmap flag. This controls the length of the total bitmap.

Which, as an aside, they're using IsFieldSet because zero-based indexes are too hard:

public bool IsFieldSet(int field) { return this.bits[field - 1]; }

But things do get worse.

/// <summary> /// Sets a field /// </summary> /// <param name="field"> /// Field to set /// </param> /// <param name="on"> /// Whether or not the field is on /// </param> public void SetField(int field, bool on) { this.bits[field - 1] = on; this.bits[0] = false; for (var i = 64; i <= 127; i++) { if (this.bits[i]) { this.bits[0] = true; break; } } }

I included the comments here because I want to highlight how useless they are. The first line makes sense. Then we set the first bit to false. Which, um, was the IsExtendedBitmap flag. Why? I don't know. Then we iterate across the back half of the bitmap and if there's anything true in there, we set that first bit back to true.

Which, by writing that last paragraph, I've figured out what it's doing: it autodetects whether you're using the higher order bits, and sets the IsExtendedBitmap as appropriate. I'm not sure this is actually correct behavior- what happens if I want to set a higher order bit explicitly to 0?- but I haven't read the spec, so we'll let it slide.

public virtual byte[] ToMsg() { var lengthOfBitmap = this.IsExtendedBitmap ? 16 : 8; var data = new byte[lengthOfBitmap]; for (var i = 0; i < lengthOfBitmap; i++) { for (var j = 0; j < 8; j++) { if (this.bits[i * 8 + j]) { data[i] = (byte)(data[i] | (128 / (int)Math.Pow(2, j))); } } } if (this.formatter is BinaryFormatter) { return data; } IFormatter binaryFormatter = new BinaryFormatter(); var bitmapString = binaryFormatter.GetString(data); return this.formatter.GetBytes(bitmapString); }

Here's our serialization method. Note how here, the length of the bitmap is either 8 or 16, while previously we were checking all the bits from 64 up to see if it was extended. At first glance, this seemed wrong, but then I realized- data is a byte[]- so 16 bytes is indeed 128 bits.

This gives them the challenging problem of addressing individual bits within this data structure, and they clearly don't know how bitwise operations work, so we get the lovely Math.Pow(2, j) in there.

Ugly, for sure. Unclear, definitely. Which only gets worse when we start unpacking.

public int Unpack(byte[] msg, int offset) { // This is a horribly nasty way of doing the bitmaps, but it works // I think... var lengthOfBitmap = this.formatter.GetPackedLength(16); if (this.formatter is BinaryFormatter) { if (msg[offset] >= 128) { lengthOfBitmap += 8; } } else { if (msg[offset] >= 0x38) { lengthOfBitmap += 16; } } var bitmapData = new byte[lengthOfBitmap]; Array.Copy(msg, offset, bitmapData, 0, lengthOfBitmap); if (!(this.formatter is BinaryFormatter)) { IFormatter binaryFormatter = new BinaryFormatter(); var value = this.formatter.GetString(bitmapData); bitmapData = binaryFormatter.GetBytes(value); } // Good luck understanding this. There be dragons below for (var j = 0; j < 8; j++) { this.bits[i * 8 + j] = (bitmapData[i] & (128 / (int)Math.Pow(2, j))) > 0; } return offset + lengthOfBitmap; }

Here, we get our real highlights: the comments. "… but it works… I think…". "Good luck understanding this. There be dragons below."

Now, John wrote this code some time ago. And the thing that I get, when reading this code, is that John was likely somewhat green, and didn't fully understand the problem in front of him or the tools at his disposal to solve it. Further, this was John's independent project, which he was doing to solve a very specific problem- so while the code has problems, I wouldn't heap up too much blame on John for it.

Which, like many other confessional Code Samples-of-the-Day, I'm sharing this because I think it's an interesting learning experience. It's less a "WTF!" and more a, "Oh, man, I see that things went really wrong for you." We all make mistakes, and we all write terrible code from time to time. Credit to John for sharing this mistake.

.comment { border: none; } [Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

Today's submitter identifies themselves as pleaseKillMe, which hey, c'mon buddy. Things aren't that bad. Besides, you shouldn't let the bad code you inherit drive you to depression- it should drive you to revenge.

Today's simple representative line is one that we share because it's not just representative of our submitter's code base, but one that shows up surprisingly often.

SELECT * FROM users WHERE last_name='NULL'

Now, I don't think this particular code impacted Mr. Null, but it certainly could have. That's just a special case of names being hard.

In this application, last_name is a nullable field. They could just store a NULL, but due to data sanitization issues, they stored 'NULL' instead- a string. NULL is not 'NULL', and thus- we've got a lot of 'NULL's that may have been intended to be NULL, but also could be somebody's last name. At this point, we have no way to know.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

It's a nearly universal experience that the era of our youth and early adulthood is where we latch on to for nostalgia. In our 40s, the music we listened to in our 20s is the high point of culture. The movies we watched represent when cinema was good, and everything today sucks.

And, based on the sheer passage of calendar time, we have a generation of adults whose nostalgia has latched onto Flash. I've seen many a thinkpiece lately, waxing rhapsodic about the Flash era of the web. I'd hesitate to project a broad cultural trend from that, but we're roughly about the right time in the technology cycle that I'd expect people to start getting real nostalgic for Flash. And I'll be honest: Flash enabled some interesting projects.

Of course, Flash also gave us Flex, and I'm one of the few people old enough to remember when Oracle tried to put their documentation into a Flex based site from which you could not copy and paste. That only lasted a few months, thankfully, but as someone who was heavily in the Oracle ecosystem at the time, it was a terrible few months.

In any case, long ago, CW inherited a Flash-based application. Now, Flash, like every UI technology, has a concept of "containers"- if you put a bunch of UI widgets inside a container, their positions (default) to being relative to the container. Move the container, and all the contents move too. I think we all find this behavior pretty obvious.

CW's co-worker did not. Here's how they handled moving a bunch of related objects around:

public function updateKeysPosition(e:MouseEvent):void{ if(dragging==1){ theTextField.x=catButtonArray[0].x-100; theTextField.y=catButtonArray[0].y-200; catButtonArray[1].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10; catButtonArray[1].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[2].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth; catButtonArray[2].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[3].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*2; catButtonArray[3].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[4].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*3; catButtonArray[4].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[5].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*4; catButtonArray[5].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[6].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*5; catButtonArray[6].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[7].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*6; catButtonArray[7].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[8].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*7; catButtonArray[8].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[9].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*8; catButtonArray[9].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[10].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+10+keyWidth*9; catButtonArray[10].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+40; catButtonArray[11].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30; catButtonArray[11].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[12].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth; catButtonArray[12].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[13].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*2; catButtonArray[13].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[14].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*3; catButtonArray[14].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[15].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*4; catButtonArray[15].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[16].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*5; catButtonArray[16].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[17].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*6; catButtonArray[17].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[18].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*7; catButtonArray[18].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[19].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+30+keyWidth*8; catButtonArray[19].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+110; catButtonArray[20].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60; catButtonArray[20].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[21].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth; catButtonArray[21].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[22].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*2; catButtonArray[22].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[23].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*3; catButtonArray[23].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[24].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*4; catButtonArray[24].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[25].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*5; catButtonArray[25].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; catButtonArray[26].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+60+keyWidth*6; catButtonArray[26].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+180; //SPACE catButtonArray[27].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+228; catButtonArray[27].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+240; //RETURN catButtonArray[28].x=catButtonArray[0].x-InitalKeyBoxWidth/2+keyWidth/2+558; catButtonArray[28].y=catButtonArray[0].y-InitalKeyBoxHeight/2+keyHeight/2+207; } } [Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

In the great olden times, when Classic ASP was just ASP, there were a surprising number of intranet applications built in it. Since ASP code ran on the server, you frequently needed JavaScript to run on the client side, and so many applications would mix the two- generating JavaScript from ASP. This lead to a lot of home-grown frameworks that were wobbly at best.

Years ago, Melinda inherited one such application from a 3rd party supplier.

<script type='text/javascript' language="JavaScript"> var NoOffFirstLineMenus=3; // Number of first level items function BeforeStart(){return;} function AfterBuild(){return;} function BeforeFirstOpen(){return;} function AfterCloseAll(){return;} // Menu tree <% If Session("SubSystem") = "IndexSearch" Then %> <% If Session("ReturnURL") = "" Then %> Menu1=new Array("Logoff","default.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Logoff"); <% else %> Menu1=new Array("<%=session("returnalt")%>","returnredirect.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Return to Application"); <% end if %> Menu2=new Array("Menu","Menu.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Menu"); Menu3=new Array("Back","","",5,20,40,"","","","","","",-1,-1,-1,"","Back to Previous Pages"); Menu3_1=new Array("<%= Session("sptitle") %>",<% If OWFeatureExcluded(Session("UserID"),"Web Index Search","SYSTEM","","")Then %>"","",0,20,130,"#33FFCC","#33FFCC","#C0C0C0","#C0C0C0","","","","","","",-1,-1,-1,"","<%= Session("sptitle") %>"); <% Else %>"SelectStorage.asp","",0,20,130,"","","","","","",-1,-1,-1,"","<%= Session("sptitle") %>"); <% End If %> Menu3_2=new Array("Indexes","IndexRedirect.asp?<%= Session("ReturnQueryString")%>","",0,20,95,"","","","","","",-1,-1,-1,"","Enter Index Search Criteria"); Menu3_3=new Array("Document List","DocumentList.asp?<%= Session("ReturnQueryString")%>","",0,20,130,"","","","","","",-1,-1,-1,"","Current Document List"); Menu3_4=new Array("Document Detail",<% If OWFeatureExcluded(Session("UserID"),"Web Document Detail",Documents.Fields.Item("StoragePlace").Value,"","") Then %>"","",0,20,135,"#33FFCC","#33FFCC","#C0C0C0","#C0C0C0","","","","","","",-1,-1,-1,"","Document Details"); <% Else %>"DetailPage.asp?CounterKey=<%= Request.QueryString("CounterKey") %>","",0,20,135,"","","","","","",-1,-1,-1,"","Document Details");<% End If %> Menu3_5=new Array("Comments","Annotations.asp?CounterKey=<%= Request.QueryString("CounterKey") %>","",0,20,70,"","","","","","",-1,-1,-1,"","Document Comments"); <% Else %> <% If Session("ReturnURL") = "" Then %> Menu1=new Array("Logoff","default.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Logoff"); <% else %> Menu1=new Array("<%=session("returnalt")%>","returnredirect.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Return to Application"); <% end if %> Menu2=new Array("Menu","Menu.asp","",0,20,100,"","","","","","",-1,-1,-1,"","Menu"); Menu3=new Array("Back","","",3,20,40,"","","","","","",-1,-1,-1,"","Back to Previous Pages"); Menu3_1=new Array("Document List","SearchDocumentList.asp?<%= Session("ReturnQueryString") %>","",0,20,130,"","","","","","",-1,-1,-1,"","Current Document List"); Menu3_2=new Array("Document Detail","DetailPage.asp?CounterKey=<%= Request.QueryString("CounterKey") %>","",0,20,135,"","","","","","",-1,-1,-1,"","Document Details"); Menu3_3=new Array("Comments","Annotations.asp?CounterKey=<%= Request.QueryString("CounterKey") %>","",0,20,70,"","","","","","",-1,-1,-1,"","Document Comments"); <% End If %> </script>

Here, the ASP code just provides some conditionals- we're checking session variables, and based on those we emit slightly different JavaScript. Or sometimes the same JavaScript, just to keep us on our toes.

The real magic is that this isn't the code that actually renders menu items, this is just where they get defined, and instead of using objects in JavaScript, we just use arrays- the label, the URL, the colors, and many other parameters that control the UI elements are just stuffed into an array, unlabeled. And then there are also the extra if statements, embedded right inline in the code, helping to guarantee that you can't actually debug this, because you can't understand what it's actually doing without really sitting down and spending time with it.

Of course, this application is long dead. But for Melinda, the memory lives on.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

Not exactly once, I sincerely hope. That would be tragic.

"Apparently, today's leap day is causing a denial of service error being able to log into our Cemetery Management software due to some bad date calculations," writes Steve D. To be fair, he points out, it doesn't happen often.

 

In all seriousness, unusual as that might be, I do have cemeteries on my mind this week. I recently discovered a web site that has photographs of hundreds of my relatives' graves, and a series of memorials for "Infant Spencer" and "Infant Strickland" and "Infant McHugh", along with another named dozen deceased age 0. Well, it's sobering. Taking a moment here in thanks to Doctors Pasteur, Salk, Jenner, et.al. And now, back to our meagre ration of snark.

Regular Peter G. found a web site that thought Lorem Ipsum was too inaccessible to the modern audience, so they translate it to English. Peter muses "I've cropped out the site identity because it's a smallish company that provides good service and I don't want to embarrass them, but I'm kinda terrified at what a paleo fap pour-over is. Or maybe it's the name of an anarcho-punk fusion group?"

 

"Beat THAT, Kasparov!" crows Orion S.

 

"Insert Disc 2 into your Raspberry Pi" quoth an anonymous poster. "I'm still looking for a way to acquire an official second installation disc for qt for Debian."

 

Finally, Michael P. just couldn't completely ignore this page, could he? "I wanted to unsubscribe to this, but since my email is not placeholderEmail, I guess I should ignore the page." I'm sure he did a yeoman's job of trying.

 

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

Brian was working on landing a contract with a European news agency. Said agency had a large number of intranet applications of varying complexity, all built to support the news business.

Now, they understood that, as a news agency, they had no real internal corporate knowledge of good software development practices, so they did what came naturally: they hired a self-proclaimed "code guru" to built the system.

Said code guru was notoriously explosive. When users came to him with complaints like "your system lost all the research I've been gathering for the past three months!" the guru would shout about how users were doing it wrong, couldn't be trusted to handle the most basic tasks, and "wiping your ass isn't part of my job description."

With a stellar personality like that, what was his PHP code like?

$req000="SELECT idfiche FROM fiche WHERE idevent=".$_GET['id_evt']; $rep000=$db4->query($req000); $nb000=$rep000->numRows(); if ($nb000>0) { while ($row000=$rep000->fetchRow(DB_FETCHMODE_ASSOC)) { $req001="UPDATE fiche SET idevent=NULL WHERE idfiche=".$row000['idfiche']; $rep001=$db4->query($req001); } }

It's common that the first line of a submission is bad. It's rare that the first 7 characters fill me with a sense of dread. $req000. Oh no. Oh noooo. We're talking about those kinds of variables.

We query using $req000 and store the result in $rep000, using $db4 to run the query. My skin is crawling so much from that that I feel like the obvious SQL injection vulnerability using $_GET to write the query is probably not getting enough of my attention. I really hate these variable names though.

We execute our gaping security vulnerability, and check how many rows we got (using $nb000 to store the result). While we have rows, we store each row in $row000, to populate $req001- an update query. We execute this query once for each row, storing the result in $rep001.

Now, the initial SELECT could return up to 4,000 rows. That's not a massive dataset, but as you can imagine, this whole application was running on a potato-powered server stuffed in the network closet. It was slow.

The fix was obvious- you could replace both the SELECT and the UPDATEs with a single query: UPDATE fiche SET idevent=NULL WHERE idevent=?- that's all this code actually does.

Fixing performance wasn't how Brian proved he was the right person for more contract work, though. Once Brian saw the SQL injection, he demonstrated to the boss how a malicious user could easily delete the entire database from the URL bar in their browser. The boss was sufficiently terrified by the prospect- the code guru was politely asked to leave, and Brian was told to please fix this quickly.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

Gabe enjoys it when clients request that he does updates on old software. For Gabe, it's exciting: you never know what you'll discover.

Public Sub AspJavaMessage(ByVal Message As String) System.Web.HttpContext.Current.Response.Write("<SCRIPT LANGUAGE=""JavaScript"">" & vbCrLf) System.Web.HttpContext.Current.Response.Write("alert(""" & Message & """)" & vbCrLf) System.Web.HttpContext.Current.Response.Write("</SCRIPT>") End Sub

This is server-side ASP .Net code.

Let's start with the function name: AspJavaMessage. We already know we're using ASP, or at least I hope we are. We aren't using Java, but JavaScript. I'm not confident the developer behind this is entirely clear on the difference.

Then we do a Response.Write to output some JavaScript, but we need to talk about the Response object a bit. In ASP .Net, you mostly receive your HttpResponse as part of the event that triggered your response. The only reason you'd want to access the HttpResponse through this long System.Web.HttpContext.Current.Response accessor is because you are in a lower-level module which, for some reason, hasn't been passed an HTTP response.

That's a long-winded way of saying, "This is a code smell, and this function likely exists in a layer that shouldn't be tampering with the HTTP response."

Then, of course, we have the ALL CAPS HTML tag, followed by a JavaScript alert() call, aka, the worst way to pop up notifications in a web page.

Ugly, awful, and hinting at far worse choices in the overall application architecture. Gabe certainly unearthed a… delightful treat.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

Let's say you had input in the form of field=value, and you wanted to pick that "value" part off. In C#, you'd likely just use String.Split and call it a day. But you're not RK's co-worker.

public string FilterArg(string value) { bool blAction; if (value.Contains('=')) blAction = false; else blAction = true; string tmpValue = string.Empty; foreach (char t in value) { if (t == '=') { blAction = true; } else if (t != ' ' && blAction == true) { tmpValue += t; } } return tmpValue; }

If the input contains an =, we set blAction to false. Then we iterate across our input, one character at a time. If the character we're on is an =, we set blAction to true. Otherwise, if the character we're on is not a space, and blAction is true, we append the current character to our output.

I opened by suggesting we were going to look at a home-grown split function, because at first glance, that's what this code looks like.

It does the job, but the mix of flags and loops and my inability to read sometimes makes it extra confusing to follow.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

"Personal Mountains" was hearing dire rumors about one of the other developers; rumors about both the quality of their work and their future prospects at the company. Fortunately for Personal Mountains, they never actually had to work with this person.

Unfortunately, that person was fired and 30,000 lines of code were now Personal Mountains' responsibility.

Fortunately, it's not really 30,000 lines of code.

list.DeleteColumn(61); list.DeleteColumn(60); list.DeleteColumn(59); list.DeleteColumn(58); list.DeleteColumn(57); list.DeleteColumn(56); list.DeleteColumn(55); list.DeleteColumn(54); list.DeleteColumn(53); list.DeleteColumn(52); list.DeleteColumn(51); list.DeleteColumn(50); list.DeleteColumn(49); list.DeleteColumn(48); list.DeleteColumn(47); list.DeleteColumn(46); list.DeleteColumn(45); list.DeleteColumn(44); list.DeleteColumn(43); list.DeleteColumn(42); list.DeleteColumn(41); list.DeleteColumn(40); list.DeleteColumn(39); list.DeleteColumn(38); list.DeleteColumn(37); list.DeleteColumn(36); list.DeleteColumn(35); list.DeleteColumn(34); list.DeleteColumn(33); list.DeleteColumn(32); list.DeleteColumn(31); list.DeleteColumn(30); list.DeleteColumn(29); list.DeleteColumn(28); list.DeleteColumn(27); list.DeleteColumn(26); list.DeleteColumn(25); list.DeleteColumn(24); list.DeleteColumn(23); list.DeleteColumn(22); list.DeleteColumn(21); list.DeleteColumn(20); list.DeleteColumn(19); list.DeleteColumn(18); list.DeleteColumn(17); list.DeleteColumn(16); list.DeleteColumn(15); list.DeleteColumn(14); list.DeleteColumn(13); list.DeleteColumn(12); list.DeleteColumn(11); list.DeleteColumn(10); list.DeleteColumn(9); list.DeleteColumn(8); list.DeleteColumn(7); list.DeleteColumn(6); list.DeleteColumn(5); list.DeleteColumn(4); list.DeleteColumn(3); list.DeleteColumn(2); list.DeleteColumn(1); list.DeleteColumn(0);

Comments in the code indicated that this was done for "extreme optimization", which leads me to believe someone heard about loop unrolling and decided to just do that everywhere there was a loop, without regard to whether or not it actually helped performance in any specific case, whether the loop was run frequently enough to justify the optimization, or understanding if the compiler might be more capable at deciding when and where to unroll a loop.

Within a few weeks, Personal Mountains was able to shrink the program from 30,000 lines of code to 10,000, with no measurable impact on its behavior or performance.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

It was an extraordinarily busy week at Error'd HQ. The submission list had an all-time record influx, enough for a couple of special edition columns. Among the list was an unusual PEBKAC. We don't get many of these so it made me chuckle and that's really all it takes to get a submission into the mix.

Headliner Lucio Crusca perseverated "Here's what I found this morning, after late night working yesterday, sitting on my couch, with my Thinkpad on my lap. No, it was not my Debian who error'd. I'm afraid it was me."

 

Eagle-eyed Ken S. called out Wells Fargo: "I see it just fine." Ditto.

 

Logical Mark W. insists "If you press 'Cancel', you are not cancelling; instead, you are cancelling the cancellation. Can we cancel this please?"

 

Peter pondered "Should I try to immediately delete, or is it safer to immediately delete?"

 

No relation to Ken S., Legal Eagle lamented "Due to my poor LTAS scores and borderline illetteracy, it was very hard for me to get into user_SchoolName. There is so much reading!" Hang in there, L. Eagle. With a resume like yours, I'm sure there will be work for you in Florida after graduation. Only the best!

 

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

I've had the misfortune to inherit a VB .Net project which started life as a VB6 project, but changed halfway through. Such projects are at best confused, mixing idioms of VB6's not-quite object oriented programming with .NET's more modern OO paradigms, plus all the chaos that a mid-project lanugage change entails. Honestly, one of the worst choices Microsoft ever made (and they have made a lot of bad choices) was trying to pretend that VB6 could easily transition into VB .Net. It was a lie that too many managers fell for, and too many developers had to try and make true.

Maurice inherited one of these projects. Even worse, the project started in a municipal IT department then was handed of to a large consulting company. Said consulting company then subcontracted the work out to the lowest bidder, who also subcontracted out to an even lower bidder. Things spiraled out of control, and the resulting project had 5,188 GOTO statements in 1321 code files. None of the code used Option Explicit (which requires you to define variables before you use them), or Option Strict (which causes errors when you misuse implicit data-type conversions). In lieu of any error handling, it just pops up message boxes when things go wrong.

Private Function getDefaultPath(ByRef obj As Object, ByRef Categoryid As Integer) As String Dim sQRY As String Dim dtSysType As New DataTable Dim iMPTaxYear As Integer Dim lEffTaxYear As Long Dim dtControl As New DataTable Const sSDATNew As String = "NC" getDefaultPath = False sQRY = "select TAXBILLINGYEAR from t_controltable" dtControl = goDB.GetDataTable("Control", sQRY) iMPTaxYear = dtControl.Rows(0).Item("TAXBILLINGYEAR") 'iMPTaxYear = CShort(cmbTaxYear.Text) If goCalendar.effTaxYearByTaxYear(iMPTaxYear, lEffTaxYear) Then End If sQRY = " " sQRY = "select * from T_SysType where MTYPECODE = '" & sSDATNew & "'" & _ " and msystypecategoryid = " & Categoryid & " and meffstatus = 'A' and " & _ lEffTaxYear & " between mbegTaxYear and mendTaxYear" dtSysType = goDB.GetDataTable("SysType", sQRY) If dtSysType.Rows.Count > 0 Then obj.Text = dtSysType.Rows(0).Item("MSYSTYPEVALUE1") Else obj.Text = "" End If getDefaultPath = True End Function

obj is defined as Object, but is in fact a TextBox. The function is called getDefaultPath, which is not what it seems to do. What does it do?

Well, it looks up the TAXBILLINGYEAR from a table called t_controltable. It runs this query by using a variable called goDB which thanks to hungarian notation, I know is a global object. I'm not going to get too upset about reusing a single database connection as a global object, but it's definitely hinting at other issues in the code.

We check only the first row from that query, which shows a great deal of optimism about how the data is actually stored in the table. While there are many ways to ensure that tables store data in sorted order, an ORDER BY clause would go a long way to making the query clear. Also, since we only need one row, a TOP N or some equivalent would be nice.

Then we use a global calendar object to do absolutely nothing in our if statement.

That leads us to the second query, which at least Categoryid is an integer and lEffTaxYear is a long, which makes this potential SQL injection not really an issue. We run that query, and then check the number of rows- a sane check which we didn't do for the last query- and then once again, only look at the first row.

I'm going to note that MSYSTYPEVALUE1 may or may not be a "default path", but I certainly have no idea what they're talking about and what data this function is actually getting here. The name of the function and the function of the function seem disconnected.

In any case, I especially like that it doesn't return a value, but directly mutates the text box, ensuring minimal reusability of the function. It could have returned a string, instead.

Speaking of returning strings, that gets us to our final bonus. It does return a string- a string of "True", using the "delightful" functionName = returnValue syntax. Presumably, this is meant to represent a success condition, but it only ever returns true, concealing any failures (or, more likely, just bubbling up an exception). The fact that the return value is a string hints at another code smell- loads of stringly typed data.

The "good" news is that what it took layers of subcontractors to destroy, Maurice's team is going to fix by June. Well, that's the schedule anyway.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

Imagine you're building a PHP web application, and you need to display different forms on different pages. Now, for most of us, we'd likely be using some framework to solve this problem, but even if we weren't, the obvious solution of "use a different PHP file for each screen" is a fairly obvious solution.

Dare I say, too obvious a solution?

What if we could have one page handle requests for many different URLs? Think of the convenience of having ONE file to run your entire application? Think of the ifs.

if( substr( $_SERVER['REQUEST_URI'], strrpos($_SERVER['REQUEST_URI'], "=" ) + 1 ) == "request" ) { echo "<form name=\"request\" action=\"\" method=\"post\" enctype=\"multipart/form-data\" onsubmit=\"return validrequest();\">\n"; } else if( substr( $_SERVER['REQUEST_URI'], strrpos($_SERVER['REQUEST_URI'], "=" ) + 1 ) == "response" ) { echo "<form action=\"\" method=\"post\" onsubmit=\"return validresponse()\">\n"; } else if( substr( substr( $_SERVER['REQUEST_URI'], stripos($_SERVER['REQUEST_URI'], "=" ) + 1 ), 0, 7 ) == "respond" ) { echo "<form name=\"respond\" action=\"\" method=\"post\" enctype=\"multipart/form-data\" onsubmit=\"return validresponse();\">\n"; } else if( substr( substr( $_SERVER['REQUEST_URI'], stripos($_SERVER['REQUEST_URI'], "=" ) + 1 ), 0, 6 ) == "upload" ) { echo "<form name=\"upload\" method=\"post\" action=\"\" enctype=\"multipart/form-data\">\n"; } else if( substr( substr( $_SERVER['REQUEST_URI'], stripos($_SERVER['REQUEST_URI'], "=" ) + 1 ), 0, 8 ) == "showitem" ) { echo "<form name=\"showitem\" action=\"\" method=\"post\" enctype=\"multipart/form-data\">\n"; } else if( substr( substr( $_SERVER['REQUEST_URI'], stripos($_SERVER['REQUEST_URI'], "=" ) + 1 ), 0, 7 ) == "adduser" ) { echo "<form name=\"adduser\" action=\"\" method=\"post\" onsubmit=\"return validadduser();\">\n"; } else if( substr( substr( $_SERVER['REQUEST_URI'], stripos($_SERVER['REQUEST_URI'], "=" ) + 1 ), 0, 8 ) == "edituser" ) { echo "<form name=\"adduser\" action=\"\" method=\"post\" onsubmit=\"return validedituser();\">\n"; } else { echo "<form action=\"\" method=\"post\">\n"; }

Someone reinvented routing, badly. We split the requested URL on an =, so that we can compare the tail of the string against one of our defined routes. Oops, no, we don't split, we take a substring, which means we couldn't have a route upload_image or showitems, since they'd collide with upload and showitem.

And yes, you can safely assume that there are a bunch more ifs that control which specific form fields get output.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

XML is, arguably, an overspecified language. Every aspect of XML has a standard to interact with it or transform it or manipulate it, and that standard is also defined in XML. Each specification related to XML fits together into a soup that does all the things and solves every problem you could possibly have.

Though Owe had a problem that didn't quite map to the XML specification(s). Specifically, he needed to parse absolutely broken XML files.

bool Sorter::Work() { if(this->__disposed) throw gcnew ObjectDisposedException("Object has been disposed"); if(this->_splitFiles) { List<Document^>^ docs = gcnew List<Document^>(); for each(FileInfo ^file in this->_sourceDir->GetFiles("*.xml")) { XElement ^xml = XElement::Load(file->FullName); xml->Save(file->FullName); long count = 0; for each(XElement^ rec in xml->Elements("REC")) { if(rec->Attribute("NAME")->Value == this->_mainLevel) count++; } if(count < 2) continue; StreamReader ^reader = gcnew StreamReader(file->OpenRead()); StringBuilder ^sb = gcnew StringBuilder("<FILE NAME=\"blah\">"); bool first = true; bool added = false; Regex ^isRecOrFld = gcnew Regex("^\\s+\\<[REC|FLD].*$"); Regex ^isEndOfRecOrFld = gcnew Regex("^\\s+\\<\\/[REC|FLD].*$"); Regex ^isMainLevelRec = gcnew Regex("^\\s+\\<REC NAME=\\\""+this->_mainLevel+"\\\".*$"); while(!reader->EndOfStream) { String ^line = reader->ReadLine(); if(!isRecOrFld->IsMatch(line) && !isEndOfRecOrFld->IsMatch(line)) continue; if(isMainLevelRec->IsMatch(line) && !String::IsNullOrEmpty(sb->ToString()) && !first) { sb->AppendLine("</FILE>"); XElement^ xml = XElement::Parse(sb->ToString()); String ^key = String::Empty; for each(XElement ^rec in xml->Elements("REC")) { key = this->findKey(rec); if(!String::IsNullOrEmpty(key)) break; } docs->Add(gcnew Document(key, gcnew XElement("container", xml))); sb = gcnew StringBuilder("<FILE NAME=\"blah\">"); first = true; added = true; } sb->AppendLine(line); if(first && !added) first = false; if(added) added = false; } delete reader; file->Delete(); } int i = 1; for each(Document ^doc in docs) { XElement ^splitted = doc->GetData()->Element("FILE"); splitted->Save(Path::Combine(this->_sourceDir->FullName, this->_docPrefix + "_" + i++ + ".xml")); delete splitted; } delete docs; } List<Document^>^ docs = gcnew List<Document^>(); for each(FileInfo ^file in this->_sourceDir->GetFiles(String::Format("{0}*.xml", this->_docPrefix))) { XElement ^xml = XElement::Load(file->FullName); String ^key = findKey(xml->Element("REC")); // will always be first element in document order Document ^doc = gcnew Document(key, gcnew XElement("data", xml)); docs->Add(doc); file->Delete(); } List<Document^>^ sorted = MergeSorter::MergeSort(docs); XElement ^sortedMergedXml = gcnew XElement("FILE", gcnew XAttribute("NAME", "MergedStuff")); for each(Document ^doc in sorted) { sortedMergedXml->Add(doc->GetData()->Element("FILE")->Elements("REC")); } sortedMergedXml->Save(Path::Combine(this->_sourceDir->FullName, String::Format("{0}_mergedAndSorted.xml", this->_docPrefix))); // returning a sane value return true; }

This is in the .NET dialect of C++, so the odd ^ sigil is a handle to a garbage collected object.

There's a lot going on here. The purpose of this function is to possibly split some pre-merged XML files into separate XML files, and then take a set of XML files and merge them back together (properly sorted).

So we start by confirming that this object hasn't been disposed, and throwing an exception if it has. Then we try and split.

To do this, we search the directory for "*.xml", and then we… load the file and then save the file? The belief about this code is that it corrects the whitespace, because later on we require some whitespace- but the .NET XML writer doesn't add whitespace, only preserve it, so I suspect this line isn't necessary- or at least shouldn't be. I can envision a world where this somehow makes the code work for reasons that are best not thought about.

Owe writes, to the preceding developers: "Thanks guys, I really appreciate this!"

Now, since we're iterating across an entire directory of XML files, some of the files have been pre-merged (and need to be unmerged), and others haven't been merged at all. How do we tell them apart? We find every element named "REC", and check if it's "NAME" attribute is equivalent to our _mainLevel value. If there are at least two such element, we know that this file has been premerged and thus needs to be unmerged.

Owe writes: "Thanks guys, I really appreciate this!"

And then we get into the dreaded parse XML with regex phase. This is done because the XML files aren't actually valid XML. So it's a mix of string operations and regex matches to try and interpret the data. And remember that whitespace that we thought we required back when we wrote the documents out? Well here's why: our regexes are matching on whitespace.

Owe writes: "Thanks guys, I really appreciate this!"

Once we've constructed all the documents in memory, we can then dump them out to a new set of files. And then, once that's done, we can reopen those files, because now the merging happens. Here we find all the "REC" elements and build new XML documents based off of them. Then a MergeSorter::MergeSort function actually does the merging- and honestly, I dread to think about what that looks like.

The merge sorter sorts the documents, but we actually want to output one document with the elements in that sorted order, so we create one last XML document, iterate across all our sorted document fragments, and then inject the "REC" elements into the output.

Owe writes: "Thanks guys, I really appreciate this!"

While the code and the entire process here is terrible, the core WTF is the "we need to store our XML with the elements sorted in a specific order". That's not what XML is for. But obviously, they don't know what XML is for, since they're doing things in their documents that can't successfully be parsed by an XML parser. Or, perhaps more accurately, they couldn't figure out how to parse as XML, hence the regexes and string munging.

Were the documents sensible, this whole thing could probably have been solved with some fairly straightforward (by XML standards) XQuery/XSLT operations. Instead, we have this. Thanks guys, I really appreciate this.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

Inheritance is one of those object-oriented concepts that creates a lot of conflicts. You'll hear people debating what constitutes an "is-a" versus a "has-a" relationship, you'll hear "favor composition of inheritance", you'll see languages adopt mix-in patterns which use inheritance to do composition. Used well, the feature can make your code cleaner to read and easier to maintain. Used poorly, it's a way to get your polymorphism with a side of spaghetti code.

Greg was working on a medical data application's front end. This representative line shows how they use inheritance:

public class Patient extends JavascriptStringBuilder

Greg writes: "Unfortunately, the designers were quite unacquainted with newfangled ideas like separating model objects from the UI layer, so they gave us this gem."

This, of course, was a common pattern in the application's front end. Many data objects inherited from string builder. Not all of them, which only helped to add to the confusion.

As for why? Well, it gave these objects a "string" function, which they could override to generate output. You want to print a patient to the screen? What could be easier than calling patient.string()?

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

An abstitution, an assortment, time travel, bad language, and an error'd.

First up, Jeremy Pereira pushes the boundaries of this column by sharing something right. "Sort of an anti-WTF. It took them 44 minutes to realise they'd made a boo-boo." They probably were notified, but it's still pretty good time to repair. Especially considering the issues we know of that last for years and years.

 

Snecod, Darren S. interjects "We had this bit of marketing from SmartSheet. The irony is that is was all about how to use tags to customise data or views."

 

Apeman Ford Prefect of Collation hooted "Perfectly (machine-)translated , except for sorting. German is called Deutsch auf Deutsch... Spanish might be Espagnol. What may be the local name of Welsh?" I know what it is, in Welsh, and it does start with C, but I don't think the original language of this list is actually Cymraeg.

 

While Wordsmith Sebas whined "Don't see how this is related." It's not.

 

Finally, junge Jurgen reflects on wording we all took for granted, until just now. "When an upload process failed, it told me to check the logs for more details. This is what I found: Prove that logs can also be recursive..."

 

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

José received a bit of VB .Net UI code that left him scratching his head.

While IsNothing(Me.FfrmWait) If Not IsNothing(Me.FfrmWait) Then Exit While End If End While

While a form doesn't exist, if the form does exist, break out of the loop. I suspect this was intended as a busy loop, waiting for some UI element to be ready. Because busy loops are always the best way to wait for things.

But even with the busy loop, the If is a puzzle- why break out of the loop early, when the loop condition is going to break itself? Did they just not understand how loops work?

Now, the last thing to note is the variable naming conventions. frmMyForm is a common convention in WinForms programming, a little bit of Hungarian notation to tell you what the UI element actually is. But what's that *leading F there? Are they… doing a double Hungarian? Does it tell us that this is a form? Or maybe they're trying to tell us that it's a field of the class? Or maybe a developer was saying "F my life"?

We'd all be better off if this code were nothing.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

It was time to start developing version 2 of Initech's flagship software product. This meant planning meetings. So many planning meetings.

The most important one, for the actual development team, was the user story meeting. The core of these meetings was a few folks from the programming team, including Steve, the director of architecture, Brian, and a variety of product owners, responsible for different segments of the overall product.

As a group, they'd review the user stories, and approve them. Once they were approved for development, work would begin.

The meetings were difficult to schedule, because of the number of stakeholders, and they were viewed as a checkpoint- you can't start implementing features until the product owner has walked through the user story with the team, but they were also a priority.

At the meeting, product owner Renee started walking the team through some of the features she owned. "So, here's my user, Fred Flinstone." Her slide popped up an image of the animated character next to a set of bullet points describing the feature. "He needs to add an item into inventory, so that it can actually be sold. To do that…"

Renee walked them through the details of the feature. Heads nodded around the table- it was a pretty straightforward CRUD application. It was good that the product owner walked them through a few workflow things unique to Initech's product, but there wasn't anything particularly shocking.

Renee advanced to the next slide. "And now, Fred needs to run a report. This report needs to…"

Again, Renee walked them through the key fields that needed to be on the report, how the report was triggered, what the reasonable filtering options would be.

"Any questions?"

Brian, the director, looked thoughtful for a moment, and then said, "I think I do. I can't really see a reason why one person would want to both add items to inventory- a receiving job- and run reports on inventory consumption. There's no reason someone doing the task would also need to run the report. I can't accept your user stories until you change one of the users to be a different person."

"What? The name doesn't matter," Renee protested.

"We should probably just stick a pin in this and pick up when we can reschedule another meeting. Thank you everyone," Brian said. He grabbed his laptop, stood, and left the meeting. Everyone sat there for a moment, realized the meeting was actually over, and followed him out a few minutes later.

That afternoon, Steve finally managed to find Brian. "What the heck was that?"

Brian sighed. "Look, the entire development team is swamped, you know it. We don't have bandwidth to take on new work. Tracey should have that priority-one bug done in a few days, and maybe once that's done we can start putting resources on the the version 2 project. For now, we need to stall, and that was the only thing I could think of."

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

As we saw yesterday, padding can be hard.

Jasmine received a set of tickets all complaining that the file download portion of an ASP .Net application took too long. Users frequently had to wait for up to 15 minutes before their browser finished downloading the output of the web application.

This particular module generated the file by doing a set of Response.Writes to build the file as part of the HTTP response. This particular line ran 200,000 times in the process of creating the file:

myResponse.Write("P;P#,##0." + new string('0', 2) + "_);;[Red]\\(#,##0." + new string('0', 2) + "\\)\n");

I'm not entirely clear what they're writing out here- it looks like some kind of format template. But the contents don't really matter. The string concatenation operations look like they're handling decimal places- 0.00. Given that the new string is hard coded to two zeros, there's no need for string concatenation, they could have just put the 00 right in the output string.

But for fun, let's count the strings that this operation creates, noting that strings are immutable in C#, so each concatenation creates a new string.

First, let's give credit to the literals- C# is smart, and string literals get only one instance created, so we can ignore those. The new string operation creates two new strings. Then each concatenation creates a new string- four.

That's a total of 6 new string instances, which isn't a lot, but given that if they just did a literal there'd be 0 new string instances, it's still a notable increase. But this line executes in a loop, 200,000 times, meaning we're spamming 1,200,000 string instances in each execution of that loop. And this line is a representative line; the whole loop has many such lines, allowing this code to quite effectively stress test the garbage collector.

Without a single change to the logic, and just switching to string formatting and string builders, Jasmine was able to get this particular file generation down from 15 minutes to closer to a few seconds.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

It's been a minute since some bad date handling code. Wesley inherited this C# blob which exists to generate timestamped filenames.

// filename format = yyyyMMddhhmmss_<bipad>.dat char c0 = '0'; this.m_FileName = DateTime.Now.Year.ToString() + new String(c0, 2-DateTime.Now.Month.ToString().Length) + DateTime.Now.Month.ToString() + new String(c0, 2-DateTime.Now.Day.ToString().Length) + DateTime.Now.Day.ToString() + new String(c0, 2-DateTime.Now.Hour.ToString().Length) + DateTime.Now.Hour.ToString() + new String(c0, 2-DateTime.Now.Minute.ToString().Length) + DateTime.Now.Minute.ToString() + new String(c0, 2-DateTime.Now.Second.ToString().Length) + DateTime.Now.Second.ToString() + "_" + new String(c0, 5-publication.Bipad.ToString().Length) + publication.Bipad.ToString() + ".dat";

The new String(c0, n-myString.Length) pattern is a perfectly cromulent way to pad a string- that is to say, completely wrong, even though it somehow communicates its intent. Even if you refuse to use a built-in pad method, writing a pad method would be good. Of course, there's no need to do any of this, as C# has date formatters that will handle generating the string for you in a single line.

Still, I have to give this credit- I had to stop and ask myself "what the hell is this even doing?" It was only for a second, but I needed to read the code twice.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!