The Daily WTF: Curious Perversions in Information Technology

2022-05-17 Reply Admin

private Guid StrToGuid(string s, int counter)
        {
            if (s == "frist") return "Boss's kid FTW";
            Guid newGuid = new Guid();
            if (counter < 10)
                Utillity.ScreenAndLog("d", s);
            try
// etc.

When I was reading this by Remy

And then you'd take a look at the method signature and see that it takes both a string, and an integer named counter, and you'd think: "Wait, what?"

My first thought was the counter was probably the amount by which the method would increment the stringly guid parameter and return a fresh guid-like stringly thing. For some very novel home-brew and certainly stringly definition of "increment" on what's really a 128 bit integer. After all, the best way to ensure uniqueness between different batches of guids created for different business purposes is to have a seed guid for each purpose, then count up from there. Right? WRONG! Totally WRONG! But far from unheard of in the wild.

2022-05-17 Reply Admin

It also accepts that counter parameter which clearly exists to log every tenth GUID we attempt to parse

The code logs < 10, not mod 10, so it'd really just log the first few GUIDs(assuming of course the counter is used properly and not just passed in magic numbers to make it log/not log as required.

Medinoc · 2022-05-17 Reply Admin

Which is a good thing here, because if you're parsing a GUID, getting a random one instead of a known sentinel value if anything goes wrong is the last thing you want.

TheCPUWizard · 2022-05-17 Reply Admin

Return either a parsed Guid for valid or Guid.Empty for invalid and keep count of the reasons why it is invalid.....

Jeremy Pereira · 2022-05-17 Reply Admin

Wait. So if I do x = new GUID(); in C# I don't actually get a globally unique identifier. That in itself is a WTF.

Jeremy Pereira · 2022-05-17 Reply Admin

I think it would be better to make the return type nullable and return null for a failure or just propagate the exception. Note that the code only handles two of the three exceptions that the constructor can throw, anyway.

2022-05-17 Reply Admin

@Jeremy Pereira I'd call it more of a trap caused by a need for higher-order consistency.

In .Net, guids are value-type structs. A feature of that sort of type is the parameterless constructor MUST create a value containing the default value of that type. For a guid = 128 bit integer, the only sensible default value is all-zeros.

There are umpteen overloads of the constructor which let you make a guid from darn near any format you might already have the corresponding bytes in from some other source. Including of course all the stringly formats Guids convert to.

https://docs.microsoft.com/en-us/dotnet/api/system.guid.-ctor?view=net-6.0

2022-05-17 Reply Admin

Yeah, it's a bit unfortunate. It makes sense if you understand the .NET type system: Guid is a value type, and the default constructor of a value type always returns the default value for that value type, i.e. all zeros. For an int (System.Int32), new int() returns 0, which kinda makes sense. new DateTime() is 0000-00-00T00:00:00Z, not the current time. It's a bit counter-intuitive in some cases, but at least it's consistent.

2022-05-17 Reply Admin

"Utillity"... harumph. I had a professor in college who docked points on assignments for any spelling errors, whether in the code or in comments. So many students gave him grief for this. I didn't; I wish more professors did this. I tire quickly of crappy documentation that's made worse by bad spelling and grammar.

2022-05-17 Reply Admin

I could see some uses for this. Let's say you are searching for valid GUIDs in a list of strings, for instance looking for files with GUIDs in them. With this code, you could keep a list of statistics. If any string is a valid GUID, you'd return it as such, otherwise as a 000...000 sentinel value.

Of course, something tells me that the statistics are meant to debug code. Something along the lines of "I'm generating a GUID with some other software as integers and then when I try to stringify it, most work, but I'm forgetting to add the leading zeros, so some don't work, hence why I'm not worried about catching the OverflowException."

2022-05-17 Reply Admin

We need something that looks like a GUID, but we don't trust the input stream. This ensures you get a GUID, although it might not be unique. The rest of it is leftover debug stuff.

This is clearly from the minimize the crashes school of programming, such things have their place but most of the time I favor crash early and often. Find and fix the problem!

2022-05-17 Reply Admin

System.Guid has no empty constructor. Never had. https://docs.microsoft.com/en-us/dotnet/api/system.guid.-ctor?view=net-6.0

So this example with Guid newGuid = new Guid(); must be a fake, it can never compile ;-)

2022-05-17 Reply Admin

No, there is no WTF - System.Guid has as a wrote above no empty constructor. And if you want an empty zero GUID you use the static property Guid.Empty. For a new guid you use the static factory method Guid.NewGuid().

2022-05-17 Reply Admin

Nevermind, I'm stupid, it's a value type and the constructor is private. Guess I never every used that one since 1.0 :-)

2022-05-18 Reply Admin

It logs when the parameter counter is less than 10. The "counter" is external to the function so one can not say when it logs, depends on caller.

2022-05-18 Reply Admin

Um, no.

You don't want a new GUID generated when you're about to parse a string. You only want an empty GUID object.

Making new GUIDs is expensive.

Steve_The_Cynic · 2022-05-18 Reply Admin

This is clearly from the minimize the crashes school of programming, such things have their place but most of the time I favor crash early and often. Find and fix the problem!

I'm with you on this. Some years back, the main guy in charge of setting "programming style" policies where I work, well, he favoured the "crash never" school, and instituted a rule that all internal APIs of our software had to report errors when given nonsense parameters, instead of assert-or-crash. That meant that:

Some functions, by their very nature, couldn't return a distinct error value for "you gave me nonsense, you idiot", so the specific error (a bug, in fact) ended up hidden.
Some functions don't have a natural error return, and thus they now have an "unnatural" error return that serves only to report "you gave me nonsense, you idiot". One of them showed up in a code review prompted by a static analyser report, and my advice was that given the nature of the arguments passed, it was impossible to give the function nonsense, so the best thing was to annotate the will to deliberately ignore the error that couldn't happen by putting (void) in front of the call.

Fortunately, the group that makes those coding rules now includes me, and doesn't include that guy, so the rule has been changed to "favour assert-or-crash". We will, over time, go back and convert as many of those old functions to "assert or crash" as we can manage.

2022-05-19 Reply Admin

Yeah, but probably not expensive enough for most people to care in most applications. You'd have to have very large volume to make the argument that initializing new guids is going to be your downfall. Otherwise the concern smells of pre-optimization.

Steve_The_Cynic · 2022-05-21 Reply Admin

You don't want a new GUID generated when you're about to parse a string.

That sounds like you're arguing for a Guid-object constructor that takes a String (or string or std::string, as appropriate) argument and parses it to produce the Guid-object. Or throws some sort of exception if the parse fails.

2022-05-24