6
\$\begingroup\$

I'm looking for input, advice, criticism. This is my first helper class. While use of mailto URLs should most often be trivial, I have a real world need to send to multiple recipients and with a body of text something more than just a one liner. Doing so gets sticky and there are some pitfalls with reserved characters, etc.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;


namespace mailto
{
    public class MailtoURL

    {

        public string Subject { get; set; }

        private string _uri = "mailto:";
        private string _toField = "?to=";
        private string _ccField = "cc=";
        private string _bccField = "bcc=";
        private string _subjectField = "subject=";
        private string _bodyField = "body=";
        private string _seperator = "&";
        private string _recipientDelimeter = ",";
        private string _linebreak = "%0D%0A";
        private string _replacePercent = "%25";
        private string _replaceAmpersand = "%26";
        private string _replaceSpace = "%20";
        private string _replaceQuestionMark = "%3F";

        private List<Recipient> Recipients;
        private List<string> BodyText;

        public MailtoURL()
        {
            Recipients = new List<Recipient>();
            BodyText = new List<string>();

        }

        public string getMailtoUrl()
        {
            StringBuilder mailtoUrl = new StringBuilder();

            mailtoUrl.Append(_uri)
                     .Append(RecipientString())
                     .Append(_seperator)
                     .Append(_subjectField)
                     .Append(Subject.Replace("%", _replacePercent).Replace("&", _replaceAmpersand).Replace(" ", _replaceSpace))
                     .Append(_seperator)
                     .Append(_bodyField)
                     .Append(BodyTextString().Replace(" ", _replaceSpace).Replace("&", _replaceAmpersand));

            return mailtoUrl.ToString();

        }

        ///<summary>Add an empty string for a new blank line</summary>
        public void AddBodyText(string bodyText)
        {   
            BodyText.Add(bodyText.Replace("%", _replacePercent) + _linebreak);

        }

        public void AddRecipient(string recipientAddress, RecipientTypes recipientType)
        {
            Recipient recipient = new Recipient();
            recipient.RecipientAddress = recipientAddress.Replace("?", _replaceQuestionMark)
                                                         .Replace("%", _replacePercent);
            recipient.RecipientType = recipientType;
            Recipients.Add(recipient);

        }

        private string BodyTextString()
        {
            var bodytext = string.Empty;

            foreach (var body in BodyText)
            {
                bodytext += body;
            }

            return bodytext;
        }

        private string RecipientString()
        {
            var recipientString = string.Empty;

            if (Recipients.Any(r => r.RecipientType == RecipientTypes.To))
            {
                foreach (var recipient in from r in Recipients where r.RecipientType == RecipientTypes.To select r)
                {
                    recipientString = _toField += recipient.RecipientAddress + _recipientDelimeter;
                }

            }

            if (Recipients.Any(r => r.RecipientType == RecipientTypes.CC))
            {
                foreach (var recipient in from r in Recipients where r.RecipientType == RecipientTypes.CC select r)
                {
                    recipientString = recipientString += _seperator + _ccField + recipient.RecipientAddress + _recipientDelimeter;
                }

            }

            if (Recipients.Any(r => r.RecipientType == RecipientTypes.Bcc))
            {
                foreach (var recipient in from r in Recipients where r.RecipientType == RecipientTypes.Bcc select r)
                {
                    recipientString = recipientString += _seperator + _bccField + recipient.RecipientAddress;
                }

            }

            return recipientString;
        }


    }
}


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace mailto

{    

    public class Recipient 
    {       
        public string RecipientAddress { get; set; }
        public RecipientTypes RecipientType { get; set; }

    }
}

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace mailto
{    
        public enum RecipientTypes
        {
            To,
            CC,
            Bcc
        }


}

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Diagnostics;

namespace mailto
{
    class Program
    {
        static void Main(string[] args)
        {
            var mailtoUrl = new MailtoURL();

            mailtoUrl.Subject = "This is my excellent &%? subject% & here is more text!";

            mailtoUrl.AddRecipient("r%eba@gmail.com", RecipientTypes.CC);
            mailtoUrl.AddRecipient("zamdrist@gmail.com", RecipientTypes.To);
            mailtoUrl.AddRecipient("joe@gmail.com", RecipientTypes.To);
            mailtoUrl.AddRecipient("j?anet@gmail.com", RecipientTypes.To);
            mailtoUrl.AddRecipient("jim@gmail.com", RecipientTypes.CC);
            mailtoUrl.AddRecipient("cory@gmail.com", RecipientTypes.Bcc);

            mailtoUrl.AddBodyText("This is my body of text & here we are.");
            mailtoUrl.AddBodyText("");
            mailtoUrl.AddBodyText("And here is yet more text. 100%!");

            Process.Start(mailtoUrl.getMailtoUrl());





        }
    }
}
\$\endgroup\$
6
\$\begingroup\$

We could probably use existing .NET classes here:

var message = new MailMessage();
message.Subject = "This is my excellent &%? subject% & here is more text!";
message.CC.Add("r%eba@gmail.com");
message.To.Add("zamdrist@gmail.com");
message.To.Add("joe@gmail.com");
message.To.Add("j?anet@gmail.com");
message.CC.Add("jim@gmail.com");
message.Bcc.Add("cory@gmail.com");

var writer = new StringWriter();
writer.WriteLine("This is my body of text & here we are.");
writer.WriteLine();
writer.WriteLine("And here is yet more text. 100%!");
message.Body = writer.ToString();

Console.WriteLine(message.ToUrl());

Where ToUrl() is our extension method:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Mail;
using static System.Uri;    

namespace System.Net.Mail
{
    public static class Mailto
    {
        public static string ToUrl(this MailMessage message) =>
            "mailto:?" + string.Join("&", Parameters(message));

        static IEnumerable<string> Parameters(MailMessage message)
        {
            if (message.To.Any())
                yield return "to=" + Recipients(message.To);

            if (message.CC.Any())
                yield return "cc=" + Recipients(message.CC);

            if (message.Bcc.Any())
                yield return "bcc=" + Recipients(message.Bcc);

            if (!string.IsNullOrWhiteSpace(message.Subject))
                yield return "subject=" + EscapeDataString(message.Subject);

            if (!string.IsNullOrWhiteSpace(message.Body))
                yield return "body=" + EscapeDataString(message.Body);
        }

        static string Recipients(MailAddressCollection addresses) =>
            string.Join(",", from r in addresses
                             select EscapeDataString(r.Address));
    }
}
\$\endgroup\$
  • \$\begingroup\$ Ah, that is a lot simpler than what I had going on. I'll have to play with that. Not sure I understand what: static IEnumerable<string> Parameters(MailMessage message) is though. \$\endgroup\$ – Steve May 29 '16 at 12:41
  • 1
    \$\begingroup\$ @StevenMpls It's an iterator, see also yield. \$\endgroup\$ – Bob May 29 '16 at 14:34
  • \$\begingroup\$ @Bob thanks, was just reading up on yield. Never seen that before. \$\endgroup\$ – Steve May 29 '16 at 14:44
  • \$\begingroup\$ StringWriter is a stream, implements IDisposable and as such should be properly disposed, or its scope narrowed to a using block; it uses a StringBuilder under the hood.... why not use the StringBuilder yourself and remove that extra overhead? \$\endgroup\$ – Mathieu Guindon May 31 '16 at 1:58
  • 1
    \$\begingroup\$ @Mat'sMug Yep, we could. In this case disposing is not required though. It is pretty common in .NET to have IDisposable implemented but not required under some scenarios. I personally prefer StringWriter as it is a way more reasonable abstraction for injection to text rendering components (supports streaming) + allows to specify FormatProvider in ctor. \$\endgroup\$ – Dmitry Nogin May 31 '16 at 2:22
6
\$\begingroup\$

Dmitry Nogin gave a great answer. Here are some ways to improve your original code that could help your coding style in the future.

  1. Those private variables holding strings should be constants.

  2. Instead of recipientString = recipientString += ... you can just write recipientString +=. I'm guessing recipientString = _toField += ... is a bug because you don't want to modify _toField.

  3. Did you test what RecipientString() does? It looks like it adds a _ccField for each CC Recipient. Same with BCC Recipients. BCC recipients aren't separated by commas, while the other two sections have commas at the end. You need to add certain things between foreach loops instead of in the foreach loops.

  4. You should use parameters instead of directly accessing class level variables (constants are okay). That way your methods will always return the same value for a given input and will be easier to test.

  5. Recipients and BodyText should follow the naming convention you use for private class level variables. I'm guessing they should be _recipients and _bodyText. CC and Bcc should have consistent capitalization.

Here is what I think is the more correct code for adding "To" Recipients:

if (Recipients.Any(r => r.RecipientType == RecipientTypes.To))
{
    recipientString += _toField;
    foreach (var recipient in from r in Recipients where r.RecipientType == RecipientTypes.To select r)
    {
        recipientString += recipient.RecipientAddress + _recipientDelimeter;
    }
    // Edit: TrimEnd() should not be used for removing a string from the end
    // of another string, even with ToCharArray().
    // recipientString.TrimEnd(_recipientDelimeter);

    // Here is a proper way to remove a string from the end of another string:
   if (inputText.EndsWith(_recipientDelimeter))
   {
       recipientString = recipientString.Substring(0, recipientString.LastIndexOf(_recipientDelimeter));
   }
}

You can combine strings with a delimiter in between by using string.Join(). It's less code, you dont' have to worry about removing the delimiter from the end, and I believe it's more efficient because constantly adding to a string in a loop creates a lot of unnecessary strings:

if (Recipients.Any(r => r.RecipientType == RecipientTypes.To))
{
    recipientString += _toField + string.Join(_recipientDelimeter, Recipients.Where(x => x.RecipientType == RecipientTypes.To);
}
\$\endgroup\$
  • \$\begingroup\$ TrimEnd expects a Char array and so I would have to use .ToCharArray(). \$\endgroup\$ – Steve May 30 '16 at 13:31
  • 1
    \$\begingroup\$ To, CC, Bcc matches the Framework's usage in System.Net.Mail \$\endgroup\$ – Steve May 30 '16 at 13:59
  • 1
    \$\begingroup\$ @StevenMpls Good catch! You probably don't want to use TrimEnd() at all. It will work with ToCharArray() in this case because the string contains only one character, but if you have more than one character, it will remove all instances of those characters from the end, even if they are in a different order. I edited in a proper way to remove a string from the end of another string. \$\endgroup\$ – Risky Martin May 30 '16 at 15:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.