tags:

views:

69

answers:

2

Hi

Sanity check of an email implementation please :-) Anything obvious I've missed?

string supplierOfThisMaterialEmailAddress = "[email protected]"; // TODO


string htmlBodyIncludingReplacements = "<html><head><title>E-mail</title></head><body><div>.  There has been a request on the hello.co.nz website for: " + txtMaterialDescription.Text +
            "<br />Full Name: <b>" + fullName + "</b><br />" +
            etc..";

        string textOnlyBodyIncludingReplacements = "E-mail.  There has been a request on the freematerials.co.nz website for: " + txtMaterialDescription.Text +
            "Full Name: " + fullName +
            "etc..";

        string subject = "Someone has contacted you";

        CustomMailer mailer = new CustomMailer();
        string result = mailer.SendEmail(subject, htmlBodyIncludingReplacements, supplierOfThisMaterialEmailAddress, textOnlyBodyIncludingReplacements);
        if (result != null)
            lblMessage.Text = result;
        else
            lblMessage.Text = "Thank you - email has been sent";

And the class:

public class CustomMailer
    {
        public string SendEmail(string subject, string htmlBodyIncludingReplacements, string emailTo, string textOnlyBodyIncludingReplacements)
        {
            try
            {
                MailAddress sender = new MailAddress("[email protected]", "Dave Mateer");
                emailTo = "[email protected]"; // testing
                MailAddress recipient = new MailAddress(emailTo, null);

                MailMessage message = new MailMessage(sender, recipient);
                message.Subject = subject;

                AlternateView textView = AlternateView.CreateAlternateViewFromString(textOnlyBodyIncludingReplacements, null, "text/plain");
                AlternateView htmlView = AlternateView.CreateAlternateViewFromString(htmlBodyIncludingReplacements, null, MediaTypeNames.Text.Html);
                message.AlternateViews.Add(textView);
                message.AlternateViews.Add(htmlView);

                SmtpClient client = new SmtpClient();
                client.Send(message);
            }

            catch (Exception ex)
            {
                throw new Exception();
            }
            return null;
        }
    }
+2  A: 

At first glance, catching a general Exception object and throwing a new one is going to have the net effect of eating any exceptions thrown by SendEmail.

The rest looks okay.

nukefusion
Many thanks everyone - wanted a sanity checked that I hadn't missed anything obvious in emailling. Appreciated.
Dave
A: 

You should change the catch (Exception ex) { throw new Exception(); } to :

catch (Exception ex)
            {
                throw;
            }

because otherwise you lose all the data that came with the original excepion that was thrown

Adibe7
...or just remove the catch block
Foole