CODEWITHSUNDEEP
c#.net
Jason Axelrod
Jason Axelrod

Reputation: 7805

How to raise exception when something goes wrong?

I am using a TRY-CATCH in my application. The try is attempting to read in an XML file. Naturally, if it fails to load the XML file, it will raise an exception. In addition to this, I have an IF statement inside the try that looks for specific nodes in the XML file; if they don't exist, I need it to fire off an exception. Is this possible?

try
{
    XmlDocument xmlDoc = new XmlDocument();
    xmlDoc.Load(dialog.FileName);

    if (xmlDoc.SelectSingleNode("event/text1") == null ||
        xmlDoc.SelectSingleNode("event/text2") == null)
    {
        FIRE EXCEPTION
    }
}
catch (Exception err)
{
    MessageBox.Show("Invalid XML file selected:\n" + err.Message);
}

Upvotes: 1

Views: 252

Answers (3)

ΩmegaMan
ΩmegaMan

Reputation: 31616

Don't throw exceptions...handle the error conditions by doing sanity checks along the way.

An actual exception, should be for, an unintended situation only. You know of the two situations, why not accommodate them?

I would refactor the code to default to error and only at the end report an error:

bool isXmlValid = false;

try
{

    if (string.IsnNullOrEmpty(dialog.FileName) == false)
    {
       var xmlDoc = new XmlDocument();
       xmlDoc.Load(dialog.FileName);

       if (xmlDoc.SelectSingleNode("event/text1") != null &&
           xmlDoc.SelectSingleNode("event/text2") != null)
       {
           // Process accordingly
           isXmlValid = true;
       }
   }
}
catch (Exception ex)
{
    Trace.WriteLine(ex.Message);
}

if (isXmlValid  == false)
      MessageBox.Show(
         string.Format("Invalid XML file or Xml structure for file ({0})", 
                       dialog.FileName));

Upvotes: 0

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149538

You should catch a narrower exception, such as an XmlException and throw a different exception if its missing a node:

try
{
    XmlDocument xmlDoc = new XmlDocument();
    xmlDoc.Load(dialog.FileName);

    if (xmlDoc.SelectSingleNode("event/text1") == null ||
    xmlDoc.SelectSingleNode("event/text2") == null)
    {
          throw new InvalidXmlException("Missing event/text node");
    }
}
catch (XmlException err)
{
    MessageBox.Show("Invalid XML file selected:\n" + err.Message);
}

Note, this is assuming you're actually throwing the exception in order to catch it higher up the stack. If you have nothing else to do with it, dont throw.

Upvotes: 3

D Stanley
D Stanley

Reputation: 152566

Why throw an exception just to catch it when all you do is show a Messagebox? Just show the message box!

try
{
    XmlDocument xmlDoc = new XmlDocument();
    xmlDoc.Load(dialog.FileName);

    if (xmlDoc.SelectSingleNode("event/text1") == null ||
        xmlDoc.SelectSingleNode("event/text2") == null)
    {
        MessageBox.Show("Invalid XML file selected.  No text1 or text node found");
    }
}
catch (Exception err)  // catch any other exception
{
    MessageBox.Show("Exception occured:\n" + err.Message);
}

If you were do re-throw the exception or do some other common activity (logging, etc.) then it might make sense, but since you don't do anything with the exception there's no sense in raising one here.

Upvotes: 1

Related Questions