The Daily WTF: Curious Perversions in Information Technology

EvanED · 2015-06-03 Reply Admin

Dlareg:
I totally agree... they should have fixed that in python 3000. It is really terrible.

IMO, "true == 1" is a disease that is the product of warped minds infected by C. ;-)

And Python 3 fixed so many other things about the type system too; they came so close. Like 5 < "string" now produces a TypeError like it should, instead of ordering things in some basically-random way.

Richard_Deeming · 2015-06-03 Reply Admin

Yes, this is obviously decompiled code.

I suspect the original code looked something like this:

public override SendMailLog SendEmail(MailInformation mailInfo, JobWorkItems recepients, bool testMode) 
{
    // Why is this initialized before validating the arguments?!
    var sendMailLog = new SendMailLog();
    sendMailLog.SendStart = DateTime.Now;
    
    if (recepients.Items.Count == 0)
    {
        // Me fail English? That's unpossible!
        throw new ArgumentNullException("recepients", "Recipient collection is empty, there is no recipients to send to.");
    }
    if (string.IsNullOrEmpty(mailInfo.From))
    {
        throw new ArgumentNullException("mailInfo", "Missing from address. SMTP servers do not allow sending without a sender.");
    }
    
    foreach (JobWorkItem current in recepients)
    {
        var mailMessage = new MailMessage(mailInfo.From, current.EmailAddress);
        mailMessage.Subject = mailInfo.Subject;
        mailMessage.Body = PersonalizeEmail(mailInfo, current.EmailAddress);
        mailMessage.IsBodyHtml = true;
        
        try
        {
            if (SendMail(mailMessage, testMode))
            {
                sendMailLog.SuccessMessages.Add(current.EmailAddress);
                current.Status = JobWorkStatus.Complete;
                
                if (current.JobId > 0)
                {
                    current.Save();
                }
            }
        }
        // Pokemon exception handling FTW!
        catch (Exception ex)
        {
            // This variable name looks auto-generated,
            // but the code suggests not.
            string str = string.Format(
                "Email: {0}\r\nException: {1}\r\n\r\n", 
                current.EmailAddress, 
                exception.Message);
            
            sendMailLog.ErrorMessages.Add(str);
            current.Status = JobWorkStatus.Failed;
            
            if (str.Length >= 2000)
            {
                str = str.Substring(0, 1999);
            }
            
            current.Info = str;
            
            if (current.JobId > 0)
            {
                current.Save();
            }
        }
    }
    
    sendMailLog.SendStop = DateTime.Now;
    return sendMailLog;
}

Still :hankey: , but not quite as bad as the decompiled version.

As for how the decompiled version ended up in the code base...

Jack_E_Wormlore · 2015-06-04 Reply Admin

That's the first thing I noticed. Reusing a variable is not a good practice, but there has been worse.

This double-negative logic though, that is priceless... and reused constantly throughout the whole procedure. Plus reassigning and testing the same variable inside the block is a double-hit combo.

Aron_Tsang · 2015-06-06 Reply Admin

csc is nothing close to a decent optimising compiler. In fact it does not even claim to be an optimising compiler. That it a job for the JITer, which may in some cases even do the opposite for allowing the processor to more easily reorder instructions.

CoyneTheDup · 2015-06-07 Reply Admin

Something finally broke free today...and I finally had to acknowledge it and now y'all just have to live with it:

One busy, two busy, three busy Booleans, Four busy, five busy, six busy Booleans, Seven busy, eight busy, nine busy Booleans, Ten busy Boolean flags.

Ten busy, nine busy, eight busy Booleans, Seven busy, six busy, five busy Booleans, Four busy, three busy, two busy Booleans, One busy Boolean flag.

Sorry.

CoyneTheDup · 2015-06-07 Reply Admin

Sorry, @Aron_Tsang, I really didn't mean to do it in reply to you, but there's DiscoReply for you. Always a zillion reply buttons visible, but never the one (you want to/should be) clicking

dkf · 2015-06-07 Reply Admin

Thank you. It saves me from having to do it.

Mouaijin · 2015-06-07 Reply Admin

Steve_The_Cynic:
And WTF, guys, a language where you have to check that an object is not NULL before deleteing it.

Yeah, technically you do in C#, because otherwise you'd be calling a method belonging to a null object. That said, you really shouldn't ever put yourself in that position, because the preferred (and sane) way to use something implementing IDisposable is to have a using(IEnumerator<JobWorkItem> enumerator = recepients.GetEnumerator()){ code goes here} statement, and never manually call Dispose(). When you go out of scope for the using statement, the compiler calls that for you. So that's its own little WTF for anyone who has used C# for anything remotely non-trivial.

Steve_The_Cynic · 2015-06-08 Reply Admin

Mouaijin:

Steve_The_Cynic:
And WTF, guys, a language where you have to check that an object is not NULL before deleteing it.
Yeah, technically you do in C#, because otherwise you'd be calling a method belonging to a null object. That said, you really shouldn't ever put yourself in that position, because the preferred (and sane) way to use something implementing IDisposable is to have a using(IEnumerator<JobWorkItem> enumerator = recepients.GetEnumerator()){ code goes here} statement, and never manually call Dispose(). When you go out of scope for the using statement, the compiler calls that for you. So that's its own little WTF for anyone who has used C# for anything remotely non-trivial.

What I was hinting at was the non-need to add ```if ( p != NULL )``` before ```delete p;``` in C++, because it is, by definition, a no-op to ```delete p;``` if ```p == NULL```...

tarunik · 2015-06-08 Reply Admin

I recall having to do that because GCC 3.4 (IIRC) was a twat about things and either complained or generated code that crashed (can't remember which) on delete NULL;...

tenshino · 2015-06-08 Reply Admin

There are cases where I might do something like that to, say, make sure something got disposed in a catch. C#, being a managed language, you don't explicitly "delete" variables. The garbage collector handles that for you. But some types might use resources that need to/should be release immediately to prevent chewing on the system's resources. In those cases, the classes should inherit from the IDisposable interface to provide a "Dispose()" method that will clean up stuff like fill handles or references to unmanaged resources. In those scenarios where you want to manually force a dispose, you have to check to make sure that you variable isn't NULL before you do that because, objectInstance.Dispose() will throw a NullReferenceException at runtime, if objectInstance is NULL.

Like Mouajin said, 95% of the time, the proper way to handle this is with a "using" statement to define the scope in which the object will live, but there are a few exceptions.

RaceProUK · 2015-06-08 Reply Admin

tenshino:
In those cases, the classes should inherit from the IDisposable interface to provide a "Dispose()" method that will clean up stuff like fill handles or references to unmanaged resources.

That's not actually a strict requirement; the `using {}` block will call any method that matches the signature `void Dispose()`.

tenshino:
In those scenarios where you want to manually force a dispose

You're doing it wrong, and should expect contact with the face of a ClueBat.

dkf · 2015-06-08 Reply Admin

tenshino:
Like Mouajin said, 95% of the time, the proper way to handle this is with a "using" statement to define the scope in which the object will live, but there are a few exceptions.

I thought you were saying (when talking about “managed language”) that the large majority of the time, you don't say anything at all about deleting the object. As long as the object isn't bridging into the unmanaged space — the massive majority of objects are like this — that's the right approach. :smile: It's only with the remaining minority that you're talking about explicit disposal, and then that's definitely best done with using where possible; scope binding is a great way to do that sort of thing provided you've got a suitable scope.

The problems only come when someone mistakes “it's a very good idea” for “it's the LAW!!!” and loses sight of the fact that some cases persistently don't fit nicely with such models.

Jaloopa · 2015-06-09 Reply Admin

RaceProUK:
That's not actually a strict requirement; the using {} block will call any method that matches the signature void Dispose().

Are you sure about that? No mention of duck typing in MSDN

Provides a convenient syntax that ensures the correct use of IDisposable objects

Edit: just checked in VS, on a .Net 4.5 project. Having a Dispose() method isn't enough, you need IDisposable

RaceProUK:

tenshino:
In those scenarios where you want to manually force a dispose
You're doing it wrong, and should expect contact with the face of a ClueBat

There are occasional cases where a `using` block doesn't make sense. In most of those, refactoring so it does makes more sense, but you don't always have the time to do that.

RaceProUK · 2015-06-09 Reply Admin

Jaloopa:
Having a Dispose() method isn't enough, you need IDisposable

Something must have changed; coulda sworn it used to just look for the method...

Jaloopa · 2015-06-09 Reply Admin

I doubt it. That would be a breaking change for any code that relied on it, and they try pretty hard to avoid that. Maybe there were proposals to add it but they never went through

Steve_The_Cynic · 2015-06-09 Reply Admin

tarunik:
I recall having to do that because GCC 3.4 (IIRC) was a twat about things and either complained or generated code that crashed (can't remember which) on `delete NULL;`...

Sounds like you were using a C-- compiler. That's a compiler that accepts C++ code, but compiles it in a way that isn't according to the standard. Almost all compilers that claim to be C++ compilers are really C-- compilers.

(Curiously, without using the option that enables trigraphs, all versions of g++ are C-- compilers, because of the notorious "slash-slash comment that ends with 3n+2 question marks and a slash" problem. With trigraphs enabled, the comment becomes 3n question marks and a backslash, which then splices the following line to the end of the comment... With trigraphs disabled, the ??/ remains ??/, and the line that follows remains a separate line.)

One (or maybe more...) version of the Sun C++ compiler didn't have the right destructor scope rules for isolated brace blocks:

{
   // code code code
   {
      SomeClassWithDestructor thingy(some,arguments,to,constructor);
      // code code code
      // ((1))
   }
   // code code code
   // ((2))
}

I'd normally expect thingy to be destructed at ((1)), but this Sun compiler destructed it at ((2)).

dkf · 2015-06-09 Reply Admin

Steve_The_Cynic:
the option that enables trigraphs

If you're writing code that requires trigraphs, you are bad and should feel bad. Or you are stuck on one of the Bad Old Platforms and should watch out for Codethulhu.

Steve_The_Cynic · 2015-06-09 Reply Admin

dkf:

Steve_The_Cynic:
the option that enables trigraphs

If you're writing code that requires trigraphs, you are bad and should feel bad. Or you are stuck on one of the Bad Old Platforms and should watch out for Codethulhu.

My point was actually that any so-called C++ compiler that doesn't compile trigraphs correctly is actually a C-- compiler. But by their default options, that means ***all*** of them...

PleegWat · 2015-06-09 Reply Admin

Steve_The_Cynic:
the notorious "slash-slash comment that ends with 3n+2 question marks and a slash" problem

:wtf:

// Why would that be a thing ????????/

Steve_The_Cynic · 2015-06-09 Reply Admin

PleegWat:
Steve_The_Cynic:
the notorious "slash-slash comment that ends with 3n+2 question marks and a slash" problem

:wtf:
// Why would that be a thing ????????/

In a sane world, it wouldn't. But with the same class of trypogaphical floo-flah that yields !!!!!!1 on a QWERTY keyboard, a line of question marks might end with a slash, on a QWERTY keyboard. On an AZERTY keyboard, exclamation is unshifted, and doesn't share with a number either, while ***both*** ? and / are shifted, but not of each other, duh, so neither of these is actually likely as an accidental accident.

dkf · 2015-06-09 Reply Admin

Steve_The_Cynic:
In a sane world, it wouldn't.

More to the point, ASCII won out over EBCDIC and the C++ spec should acknowledge this by removing trigraphs. Minimally from the required feature set, but preferably from the current timeline by going back in time and killing the grandfather of their inventor. It's the only way to be sure…

EvanED · 2015-06-09 Reply Admin

Steve_The_Cynic:

I recall having to do that because GCC 3.4 (IIRC) was a twat about things and either complained or generated code that crashed (can't remember which) on delete NULL;...

Sounds like you were using a C-- compiler.

Or (well, really, "Perhaps and") one that uses a buggy C library. Some crash if you `free(NULL)`, which is also legal per the standard but not all implementations match.

dkf:
the C++ spec should acknowledge this by removing trigraphs

I actually thought this was done in C++11, but it wasn't (courtesy of... IBM, surprise!). Probably will be done in C++17 though.

tenshino · 2015-06-09 Reply Admin

I was under the impression IDisposable was a requirement for the using.

There have been rare cases where I have had to refactor existing code, without "breaking" the current implementation. For example, adding a method overload to include a Linq2Sql DataContext as a parameter because we now need to know the original context for something, but I don't want to have to track down every line of code that was calling the original method signature. All the code gets moved to the new method, with a try/finally; original method now just calls the new signature with a "null" for the context parameter. First thing we do under the new method signature is to check and see if the passed DataContext is null and instantiating a new one if it is. If we created a local context, set a bool value to "true" so that, inside of a "finally" block, I can dispose the context only if it was created within the method itself.

They're rare "dirty hack" scenarios that are sometimes unavoidable when you're dealing with a system that has a half-million lines of code, a tight deadline, and a bad PM who keeps agreeing to all kinds of changes without adjusting the deadline.

RaceProUK · 2015-06-09 Reply Admin

tenshino:
I was under the impression IDisposable was a requirement for the using.

I coulda sworn I read an Eric Lippert thing where he said you only need the method; I guess I was mistaken. Not that I can find that article anyway :smile:

tenshino · 2015-06-09 Reply Admin

It's also possible that there is some scenario under which you are entirely correct and I just don't know about it :smile:

It's not like it would be difficult to check for a "Dispose()" method on a class using reflection.

RaceProUK · 2015-06-09 Reply Admin

tenshino:
It's not like it would be difficult to check for a "Dispose()" method on a class using reflection.

You wouldn't even need that; all the compiler does is convert the using {} to a

{
    O o;
    try
    {
        o = new O();
    }
    finally
    {
        if (o != null) o.Dispose();
    }
}

OK, the true transformation is a little more robust, but that's the gist of it. And if there's no Dispose(), compiler error :smile:

tenshino · 2015-06-09 Reply Admin

True, but you're actually overcomplicating it a bit:

public class MyClass : IDisposable
{
    public MyClass()
    {
        // Class Constructor
    }
    public void Dispose()
    {
        // Do dispose-type stuff...
    }
}
static class Program
{
    void Main()
    {
        Object obj = new MyClass();
        obj.Dipose();
    }
}

^^^ Compiler error: 'Object' does not contain a 'Dispose' method.

It's the same result as what you're proposing. With the IDisposable interface inheritance, I can just do this:

((IDisposable)obj).Dispose();

... and that will compile and run just fine. I'd rather use the interface method ;)

Of course, to avoid the cluebat to the face, I will point out that the proper way to do this would be:

using(MyClass obj = new MyClass())
{
    // Do stuff with the 'obj' instance of MyClass here.
}

Which is much cleaner than any of the other options :)

RaceProUK · 2015-06-09 Reply Admin

I… :headdesk:

I've had my head in NodeJS too long :smile:

tenshino · 2015-06-09 Reply Admin

lol. I'm trying (time permitting) to learn NodeJS right now. It's cool, but definitely a very different animal from C#.

FrostCat · 2015-06-09 Reply Admin

RaceProUK:
I coulda sworn I read an Eric Lippert thing where he said you only need the method; I guess I was mistaken. Not that I can find that article anyway

You got it backwards, it looks like:

http://blogs.msdn.com/b/ericlippert/archive/2011/06/30/following-the-pattern.aspx

"Why require that an object implement IDisposable to be disposed in a "using" block? Why not just require that it implement a public method called Dispose?"

another_sam · 2015-06-10 Reply Admin

tenshino:
All the code gets moved to the new method, with a try/finally; original method now just calls the new signature with a "null" for the context parameter. First thing we do under the new method signature is to check and see if the passed DataContext is null and instantiating a new one if it is. If we created a local context, set a bool value to "true" so that, inside of a "finally" block, I can dispose the context only if it was created within the method itself.

You're a bad person.

oldMethod(blah)
{
    using (context = new context())
    {
        implMethod(blah, context);
    }
}

newMethod(blah, context)
{
    implMethod(blah, context);
}

implMethod(blah, context)
{
    // whatever
}

obeselymorbid · 2015-06-10 Reply Admin

Steve_The_Cynic:
accidental accident.

As opposed to deliberate "accident"?

<SCNR

Maciejasjmj · 2015-06-10 Reply Admin

Steve_The_Cynic:
But with the same class of trypogaphical floo-flah that yields !!!!!!1 on a QWERTY keyboard, a line of question marks might end with a slash, on a QWERTY keyboard.

If you mistake your code window with your FB Messenger window, you're in deeper shit than the trigraphs will ever put you in anyway.

accalia · 2015-06-10 Reply Admin

RaceProUK:
all the compiler does is convert the using {} to a

well it's a bit more complicated than that because it does it direct into IL instead of C# and then reinterpreting, but yes that is the moral equivalent of the syntactic sugar.

tiny pedantry i know, but it had to be said.

accalia · 2015-06-10 Reply Admin

RaceProUK:
I've had my head in NodeJS too long

sounds about the right amount of time actually...

:fa_trophy:

Steve_The_Cynic · 2015-06-10 Reply Admin

Maciejasjmj:

Steve_The_Cynic:
But with the same class of trypogaphical floo-flah that yields !!!!!!1 on a QWERTY keyboard, a line of question marks might end with a slash, on a QWERTY keyboard.

If you mistake your code window with your FB Messenger window, you're in deeper shit than the trigraphs will ever put you in anyway.

True, but I'm thinking of a QWERTYist who, for whatever reason, wants a long line of question marks, and releases Shift a character early, in the same way that "!!!!!!1" is produced by letting go of Shift a character early when typing lots of shrieks.

And, of course, nobody picked up on my error, the one about 3n+2 question marks. ??? isn't a valid trigraph, so only the ??/ at the end matters, and therefore I should have said "at least two question marks"...

Steve_The_Cynic · 2015-06-10 Reply Admin

obeselymorbid:

Steve_The_Cynic:
accidental accident.

As opposed to deliberate "accident"?

<SCNR

Exactly.

Maciejasjmj · 2015-06-10 Reply Admin

Steve_The_Cynic:
who, for whatever reason, wants a long line of question marks

Name one valid reason other than "being an idiot".

The Busy Little Boolean

Leave a comment on “The Busy Little Boolean”