Article 6K8FN CodeSOD: Some Original Code

CodeSOD: Some Original Code

by
Remy Porter
from The Daily WTF on (#6K8FN)

FreeBSDGuy sends us a VB .Net snippet, which layers on a series of mistakes:

If (gLang = "en") Then If (item.Text.Equals("Original")) Then item.Enabled = False End IfElseIf (gLang = "fr") Then If (item.Text.Equals("Originale")) Then item.Enabled = False End IfElse If (item.Text.Equals("Original")) Then item.Enabled = False End IfEnd If

The goal of this code is to disable the "original" field, so the user can't edit it. To do this, it checks what language the application is configured to use, and then based on the language, checks for the word "Original" in either English or French.

The first obvious mistake is that we're identifying UI widgets based on the text inside of them, instead of by some actual identifier.

As an aside, this text sure as heck sounds like a label which already doesn't allow editing, so I think they're using the wrong widget here, but I can't be sure.

Then we're hard-coding in our string for comparison, which is already not great, but then we are hard-coding in two languages. It's worth noting that .NET has some pretty robust internationalization features that help you externalize those strings. I suspect this app has a lot of if (gLang = "en") calls scattered around, instead of letting the framework handle it.

But there's one final problem that this code doesn't make clear: they are using more unique identifiers to find this widget, so they don't actually need to do the If (item.Text.Equals("Original")) check. FreeBSDGuy replaced this entire block with a single line:

 item.Enabled = False
buildmaster-icon.png [Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!
External Content
Source RSS or Atom Feed
Feed Location http://syndication.thedailywtf.com/TheDailyWtf
Feed Title The Daily WTF
Feed Link http://thedailywtf.com/
Reply 0 comments