What should you do if assigning is going to crash your application?

Vladimir Svyatski

Another coding horror. Have you ever thought that usual assigning operation, like a=b, can crash your application? Not yet? You gotta be ready. One of my teammates has already prepared:

public class InspectionMethod
{
    private string name\_;

    \[XmlElementAttribute()\]
    public string Name
    {
        get
        {
            return name\_;
        }
        set
        {
            name\_ = value;
        }
    }

    //This method is the most interesting part
    public void Resuscitate()
    {
        try
        {
            name\_ = this.Name;
        }
        catch (Exception ex)
        {
            MessageBox.Show(ex.Message, "Serialization error has happened.",
              MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
        }
    }
}

And indeed, never forget about message boxes. User must be warned, right?

Julien Villers

Especially nice considering the getter... This translates to a fancy "name_ = name_", well done :thumbsup:

'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood 'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail

Vladimir Svyatski

I didn't spotted that initially (perhaps I was way too attracted by try/catch). But now I've fallen from my chair, thank you. :laugh:

Julien Villers

I think this is incomplete. You should add this in the try block:

if (this.Name != name_)
throw new InvalidOperationException("Bad XML !");

'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood 'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail

Lost User

And before that:

if (this == null)
{
throw new NullReferenceException();
}

And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"

And I smiled and was happy
And it came worse.

Julien Villers

Oh yes, silly me! I forgot to check that case :D

'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood 'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail

Lost User

I'm just wondering in how many ways this is useless...

And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"

And I smiled and was happy
And it came worse.

dawmail333

Don't forget this fella:

Microsoft.Win32.SystemEvents.SessionEnding += (a,b) =>
{
MessageBox.Show("An expected error occurred.");
}

Don't forget to rate my post if it helped! ;) "He has no enemies, but is intensely disliked by his friends." "His mother should have thrown him away, and kept the stork." "There's nothing wrong with you that reincarnation won't cure." "He loves nature, in spite of what it did to him."

YvesDaoust

Won't ((InspectionMethod*)BadPointer)->Resuscitate() raise the exception ?

Lost User

And this, at least if we have multithreaded code:

catch(ThreadAbortException x)
{
throw;
}

For one thing,it would lessen the appearance of doing Pokemon exception handling (You must catch them all!) and it may have strange side effects to let this exception simply disappear.

And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"

And I smiled and was happy
And it came worse.

BobJanova

While this is obviously dumb code, in C# and Delphi (and any other language with property support like VB.net), a 'simple assignment' is essentially calling a method so it is as possible to have crash-inducing code in there as it is anywhere else. Particularly if it's something like "myReallyComplicatedRemotingThingy.Connected = true".

Vladimir Svyatski

It's not only Pokemon exception handling, Smurf naming convention is also widely used in the application I took the snippet from. I just removed it from class name, because it leads in some way to the source of the snippet.

dawmail333

The source code for the property is included. If that causes a crash, you have bigger problems.

Don't forget to rate my post if it helped! ;) "He has no enemies, but is intensely disliked by his friends." "His mother should have thrown him away, and kept the stork." "There's nothing wrong with you that reincarnation won't cure." "He loves nature, in spite of what it did to him."

BobJanova

Yes, obviously in the posted example the developer is just an idiot. Just saying, an assignment sign doesn't automatically make a line of code crash-proof. Even a property getter (which looks totally innocuous) can break stuff, though one can argue that that is bad style in itself (Microsoft do it though, for example trying to read Length out of an unbounded stream).

TorstenFrings

too sad you can't override the assignment operator in c#

KP Lee

I love the error message!

VUnreal wrote:

MessageBox.Show(ex.Message, "Serialization error has happened.", MessageBoxButtons.OK, MessageBoxIcon.Exclamation);

That's assuming an error is thrown. (I've seen the computer do impossible things several times, so I wouldn't put it past it.) Serialization isn't happening here. No information about where it is happening. Oh shoot, it has ex.Message in it. at first I thought it wouldn't even tell you what was wrong. Oh, lets throw in an icon to show this is an important error message. The name of the routine is interesting. You're in a type safe environment that has an invalid type passed into the string and you are going to "fix" it by passing the invalid type back into the string. Really "advanced" thinking going on here.

VallarasuS

CDP1802 wrote:

if (this == null) { throw new NullReferenceException(); }

You don't know how hard we laughed when we found such one in code library.

Regards Vallarasu S | BreakingDotNet.blogspot.com