CodeSOD: GUID Enough, I Guess

Philemon Eichin has a depressing problem: his codebase is full of WTFs and git blame always returns his name. It's not his fault! Before he joined the team, "source control" was "a zip file named Project.Final.Real.Finished.CurrentRelease.zip.

Periodically, he'll trawl through the codebase, tracking down ugly, bad code and fixing it, as a way to cut down on how many WTFs are attached to his name.

For some reason, and Philemon isn't sure, he didn't fix these methods. He just slapped an Obsolete annotation on them, which explains what he should have done. Maybe he didn't want to fight with regressions, since there weren't any unit tests. Maybe he just liked the logging output. Regardless, this remains in his codebase:

 // Konvertierung einer z.B. im Oracle-Server als // VARCHAR2(32) gespeicherten Guid in ein fi1/4r den // System.Guid Datentyp (z.B. im SQL-Server) gi1/4ltiges // Beispiel: // SQL-Server: {ADF60E63-0D32-4423-B044-631E08093E3C} // Oracle-Server: ADF60E630D324423B044631E08093E3C [Obsolete("Replace with Guid.Parse")] public static Guid GuidFromString(string value, ILog log = null) { Guid trgValue = Guid.Empty; try { string newId = $"{value.Substring(0, 8)}-{value.Substring(8, 4)}-{value.Substring(12, 4)}-{value.Substring(16, 4)}-{value.Substring(20)}"; trgValue = new Guid(newId); log.DebugFormat("GuidFromString() In:<{0}> Out:<{1}>", value, trgValue); } catch (Exception ex) { log.Error("String => Guid Conversion failed!", ex); } return trgValue; } // Konvertierung des System.Guid Datentyps (z.B. im SQL-Server) // in einen (z.B. im Oracle-Server verwendbaren) String mit // fester Linge von 32-Zeichen, z.B. als (VARCHAR2(32) // Beispiel: // IN: {ADF60E63-0D32-4423-B044-631E08093E3C} [SQL-Server] // OUT: ADF60E630D324423B044631E08093E3C [Oracle-Server] [Obsolete("Replace with guid.ToString(N).ToUpper()")] public static string GuidToString(Guid value, ILog log) { string s = value.ToString(); s = s.Replace("{", "").Replace("-", "").Replace("}", "").ToUpper(); string trgValue = s; log.DebugFormat("GuidToString() In:<{0}> Out:<{1}>", value, trgValue); return trgValue; }

Now, if you skim through this code, it's not an utter disaster. As the Obsolete annotation points out, you could replace the entire body with a built-in method, but they don't take any obviously terrible approaches to reinvent a wheel which already exists. They didn't blunder onto the absolute worst way to format/parse GUIDs, they just" wrote pointless code.

So, what's the WTF?

Well, I'm no German expert, but if you read the comments it's pretty clear that this code exists because SQL Server and Oracle use two different formats for GUIDs/UUIDs. Now, sure, Guid.Parse and Guid.ToString could handle both formats just fine, so this code remains unnecessary- but doubly so.

Because this product doesn't talk to an Oracle database. It never has. It never will. Much of the business logic lives in stored procedures. It absolutely never will talk to Oracle. So the original developer solved a problem they'll never have, by re-implementing built-in methods.

Compared to that, just marking them as Obsolete and not fixing the bad code is a very minor sin. It's good enough, it just shouldn't exist is all.

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