patterncsharpModerate
Application please send me an E-mail
Viewed 0 times
pleasesendmailapplication
Problem
This is an E-mailer class that I created to send me E-mail when/if the Windows service (that will be installed on computers all over) throws an error, so that I know what is going on.
I use a similar set up all over the place, so I would like to know if this is clean and efficient.
I would also like to know if I have the right ideas about how classes should work and function; this is just a simple method wrapped inside of a class. Is this how it should be?
I use a similar set up all over the place, so I would like to know if this is clean and efficient.
class Emailer
{
public void sendEmail(string bodyText, string sender, List recipientAddresses)
{
int portNumber = 25;
SmtpClient smtpClient = new SmtpClient("mailexchanger_String", portNumber);
using (MailMessage mailMsg = new MailMessage())
{
mailMsg.Body = bodyText;
mailMsg.IsBodyHtml = true;
mailMsg.Priority = MailPriority.High; //I want to know now!
foreach (string address in recipientAddresses)
{
mailMsg.To.Add(address);
}
mailMsg.From = new MailAddress(sender);
smtpClient.Send(mailMsg);
}
}
}I would also like to know if I have the right ideas about how classes should work and function; this is just a simple method wrapped inside of a class. Is this how it should be?
Solution
Some thoughts about the code itself.
The modified code might look like:
You can also configure an SMTP client via the
you do not need to provide the connection details in the code but instead can configure it via the .config file.
For an MVC.net application that may be in the
This means that you could create the SMTP client on Emailer creation and then dispose it as necessary. In this case the code might look like
And used like:
- In general method names recommended as Pascal Case so
sendEmailshould ideally be namedSendEmail
- I might consider trying to make the class a bit more configurable so that it could be extended in more ways without being modified. For example if you wanted to send a text email, or you wanted to send an email using a different server.
The modified code might look like:
public class Emailer
{
// Required for emailer to work
private readonly string _host;
private readonly int _portNumber;
// Optional configurable options
public bool IsHtml { get; set; }
public MailPriority Priorty { get; set; }
public Emailer()
: this("mailexchanger_String", 25)
{
}
public Emailer(string host, int portNumber)
{
_host = host;
_portNumber = portNumber;
IsHtml = true;
Priorty = MailPriority.High;
}
public void SendEmail(
string bodyText,
string sender,
List recipientAddresses)
{
SendEmail(
bodyText,
new MailAddress(sender),
recipientAddresses.Select(p => new MailAddress(p)).ToList());
}
public void SendEmail(
string bodyText,
MailAddress sender,
List recipientAddresses)
{
// .NET 4.0 only for SmtpClient disposing
using (var smtpClient = new SmtpClient(_host, _portNumber))
{
using (var mailMsg = new MailMessage())
{
mailMsg.From = sender;
mailMsg.Body = bodyText;
mailMsg.IsBodyHtml = IsHtml;
mailMsg.Priority = Priorty;
recipientAddresses.ForEach(p => mailMsg.To.Add(p));
smtpClient.Send(mailMsg);
}
}
}
}You can also configure an SMTP client via the
.config. In that caseyou do not need to provide the connection details in the code but instead can configure it via the .config file.
For an MVC.net application that may be in the
web.config or for a desktop application that might be in an app.config.
">
This means that you could create the SMTP client on Emailer creation and then dispose it as necessary. In this case the code might look like
internal class Emailer : IDisposable
{
// Our smtp client that will be disposed
private readonly SmtpClient _smtpClient;
// Optional arguments
private bool _isHtml = true;
public bool IsHtml { get { return _isHtml; } set { _isHtml = value; } }
private MailPriority _mailPriority = MailPriority.High;
public MailPriority Priorty { get { return _mailPriority; } set { _mailPriority = value; } }
public Emailer()
{
_smtpClient = new SmtpClient();
}
public Emailer(string host, int portNumber)
{
_smtpClient = new SmtpClient(host, portNumber);
}
public void SendEmail(
string bodyText,
string sender,
List recipientAddresses)
{
SendEmail(
bodyText,
new MailAddress(sender),
recipientAddresses.Select(p => new MailAddress(p)).ToList());
}
public void SendEmail(
string bodyText,
MailAddress sender,
List recipientAddresses)
{
using (var mailMsg = new MailMessage())
{
mailMsg.From = sender;
mailMsg.Body = bodyText;
mailMsg.IsBodyHtml = IsHtml;
mailMsg.Priority = Priorty;
recipientAddresses.ForEach(p => mailMsg.To.Add(p));
_smtpClient.Send(mailMsg);
}
}
public void Dispose()
{
_smtpClient.Dispose(); // Or not if using < .NET 4.0
}
}And used like:
using(var emailer = new Emailer())
{
emailer.SendEmail(
bodyText,
sender,
"sendtome"
);
}Code Snippets
public class Emailer
{
// Required for emailer to work
private readonly string _host;
private readonly int _portNumber;
// Optional configurable options
public bool IsHtml { get; set; }
public MailPriority Priorty { get; set; }
public Emailer()
: this("mailexchanger_String", 25)
{
}
public Emailer(string host, int portNumber)
{
_host = host;
_portNumber = portNumber;
IsHtml = true;
Priorty = MailPriority.High;
}
public void SendEmail(
string bodyText,
string sender,
List<string> recipientAddresses)
{
SendEmail(
bodyText,
new MailAddress(sender),
recipientAddresses.Select(p => new MailAddress(p)).ToList());
}
public void SendEmail(
string bodyText,
MailAddress sender,
List<MailAddress> recipientAddresses)
{
// .NET 4.0 only for SmtpClient disposing
using (var smtpClient = new SmtpClient(_host, _portNumber))
{
using (var mailMsg = new MailMessage())
{
mailMsg.From = sender;
mailMsg.Body = bodyText;
mailMsg.IsBodyHtml = IsHtml;
mailMsg.Priority = Priorty;
recipientAddresses.ForEach(p => mailMsg.To.Add(p));
smtpClient.Send(mailMsg);
}
}
}
}<system.net>
<mailSettings>
<smtp from="<fromaddress>">
<network host="mailexchanger_String" password="??" userName="??" port="25" enableSsl="true" defaultCredentials="false" />
</smtp>
</mailSettings>
</system.net>internal class Emailer : IDisposable
{
// Our smtp client that will be disposed
private readonly SmtpClient _smtpClient;
// Optional arguments
private bool _isHtml = true;
public bool IsHtml { get { return _isHtml; } set { _isHtml = value; } }
private MailPriority _mailPriority = MailPriority.High;
public MailPriority Priorty { get { return _mailPriority; } set { _mailPriority = value; } }
public Emailer()
{
_smtpClient = new SmtpClient();
}
public Emailer(string host, int portNumber)
{
_smtpClient = new SmtpClient(host, portNumber);
}
public void SendEmail(
string bodyText,
string sender,
List<string> recipientAddresses)
{
SendEmail(
bodyText,
new MailAddress(sender),
recipientAddresses.Select(p => new MailAddress(p)).ToList());
}
public void SendEmail(
string bodyText,
MailAddress sender,
List<MailAddress> recipientAddresses)
{
using (var mailMsg = new MailMessage())
{
mailMsg.From = sender;
mailMsg.Body = bodyText;
mailMsg.IsBodyHtml = IsHtml;
mailMsg.Priority = Priorty;
recipientAddresses.ForEach(p => mailMsg.To.Add(p));
_smtpClient.Send(mailMsg);
}
}
public void Dispose()
{
_smtpClient.Dispose(); // Or not if using < .NET 4.0
}
}using(var emailer = new Emailer())
{
emailer.SendEmail(
bodyText,
sender,
"sendtome"
);
}Context
StackExchange Code Review Q#62785, answer score: 11
Revisions (0)
No revisions yet.