C# helper class - mailto

Question

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%[email protected]", RecipientTypes.CC);
            mailtoUrl.AddRecipient("[email protected]", RecipientTypes.To);
            mailtoUrl.AddRecipient("[email protected]", RecipientTypes.To);
            mailtoUrl.AddRecipient("[email protected]", RecipientTypes.To);
            mailtoUrl.AddRecipient("[email protected]", RecipientTypes.CC);
            mailtoUrl.AddRecipient("[email protected]", 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());





        }
    }
}

Answer 1

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%[email protected]");
message.To.Add("[email protected]");
message.To.Add("[email protected]");
message.To.Add("[email protected]");
message.CC.Add("[email protected]");
message.Bcc.Add("[email protected]");

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));
    }
}

Answer 2

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);
}