Passing parameters to functions and methods in a proper way, forcing sequence of events by using parameters.

Following my last article (Creating objects and basic data input validation), where we analyzed basic input parameters at the level of constructor, I think we should investigate the subject of passing parameters and parameter validation in functions and methods.

By the way I call something a function if it does not alter any data, only calculates something based on input and returns other data. Functions are perfect to be static and public, so they can be reusable and easily testable.

Let’s create a class that makes it easy to send an email. Following recommendations from previous article we should have read only fields for data initialized in constructor. Also we should validate input before it gets processed by any methods and functions.

   class EmailSender
    {      
        readonly string fromString;
        readonly string recipientString;
        readonly string body;
        readonly string subject;
        public EmailSender(string FromString, string RecipientString, string Body, string Subject)
        {
            if (String.IsNullOrEmpty(FromString)) 
              throw new ArgumentNullException(nameof(FromString));
            if (String.IsNullOrEmpty(RecipientString)) 
              throw new ArgumentNullException(nameof(RecipientString));
            if (String.IsNullOrEmpty(Body)) 
              throw new ArgumentNullException(nameof(Body));
            if (String.IsNullOrEmpty(Subject)) 
              throw new ArgumentNullException(nameof(Subject));

            this.fromString = FromString;
            this.recipientString = RecipientString;
            this.body = Body;
            this.subject = Subject;
        }

Then we need public Send method and internal Send method. Public method is exposed for class users, it allows passing additional parameters and SendInternal method is the real smtp sent method.

        public bool Send(string Host, int Port)
        {
            if (String.IsNullOrEmpty(Host)) throw new   
              ArgumentNullException(nameof(Host));
            if (Port<=0 || Port>= 65000) throw new 
              ArgumentNullException(nameof(Port));

            List<MailAddress> recipients = 
               TransformRecipients(recipientString);
            MailAddress fromAddress = new MailAddress(fromString);
            return SendInternal(recipients, fromAddress, Host, Port);
        }

        private bool SendInternal(List<MailAddress> Recipients, MailAddress From, string Host, int Port)
        {
            try
            {
                SmtpClient client = new SmtpClient(Host, Port);
                foreach (var recipient in Recipients)
                {
                    var message = new MailMessage(From, recipient);
                    message.Subject = subject;
                    message.Body = body;
                    client.Send(message);
                }
                return true;
            }
            catch (Exception ex)
            {
                return false;
            }
        }

        public static List<MailAddress> TransformRecipients(string 
           recipients)
        {
            List<MailAddress> mailList = new List<MailAddress>();
            var list= recipients.Split(',').ToList();
            foreach (var element in list)
                mailList.Add(new MailAddress(element));
            return mailList;
        }

TransformRecipients function just converts a list of comma separated email addresses into a List<MailAddress>.

All nice and clear ? No.

This was just an example of bad code at conceptual level. It works more or less but has bad design. As you can see Host and Port are parameters of public Send function. But it should be opposite: constructor should receive Host and Port, and public Send should receive mail details like body and subject. This is because we typically send multiple emails to one host and not vice versa. Let’s correct our code:

 class EmailSender2
 {
        List<MailAddress> recipients;
        MailAddress fromAddress;
        readonly SmtpClient client;
        string subject;
        string body;

        public EmailSender2(String Host, int Port)
        {
            if (String.IsNullOrEmpty(Host)) throw new ArgumentNullException(nameof(Host));
            if (Host.Contains("localhost")) throw new ArgumentException("Host cannot be localhost");
            if (Port <= 24 || Port >= 20000) throw new ArgumentNullException(nameof(Port));
            client = new SmtpClient(Host, Port);
        }

        public bool Send(string From, string Recipients, string Body, string Subject)
        {
            if (String.IsNullOrEmpty(From)) throw new ArgumentNullException(nameof(From));
            if (String.IsNullOrEmpty(Recipients)) throw new ArgumentNullException(nameof(Recipients));
            if (String.IsNullOrEmpty(Body)) throw new ArgumentNullException(nameof(Body));
            if (String.IsNullOrEmpty(Subject)) throw new ArgumentNullException(nameof(Subject));

            recipients = TransformRecipients(Recipients);
            fromAddress = TransformOneRecepient(From);
            body = Body;
            subject = Subject;
            
            return SendInternal();
        }

So now we have proper validation of smtp host data in the constructor and validation of email input details in Send method. We could even move all parameters to constructor if this EmailSender class was only for one email per call, but then it makes no sense to expose Send as public method as it is not reusable (it would just send a copy of same email to same person). In such case I would suggest exposing static Send method and nothing else.

In our case we want to have one class to send many emails to one host, so we have a sequence: 1) construct EmailSender object with smtp host data 2) validate email details and send emails. Let’s look at public Send method. It checks all input parameters which is good. Also it tries to convert From and Recipient email addresses from string to MailAddress, which is also good. In case of validation exception we would not try to send anything.

But what’s interesting is that SendInternal method in this example does not get any parameters at all. Looks much cleaner. But how does it know where to send email and with what body and subject ? We just assigned input parameters to private fields of class so the object EmailSender knows all required info.

Is our new version of EmailSender good now ? No.

It’s tempting to remove parameters in internal functions , code is easier to read and cleaner. Unfortunately it leads to a lot of mess and errors. In most cases you should assign input parameters to class private fields only in constructor. It makes it easier to track bugs if there is only one place for validation of data. Other problem is that without required parameters in SendInternal method you lose sequence. By sequence I mean that by using required parameters you force correct sequence of events in your class. In EmailSender we need to transform input data into acceptable form and THEN use SendInternal. SendInternal without parameters could be misunderstood and you or somebody else could remove all those assignments for recepients, from, body and subject and SendInternal would fail.

So correct class should look like this:

    class EmailSender3
    {
        readonly SmtpClient client;

        public EmailSender3(String Host, int Port)
        {
            if (String.IsNullOrEmpty(Host)) throw new ArgumentNullException(nameof(Host));
            if (Host.Contains("localhost")) throw new ArgumentException("Host cannot be localhost");
            if (Port <= 24 || Port >= 20000) throw new ArgumentNullException(nameof(Port));
            client = new SmtpClient(Host, Port);
        }

        public bool Send(string FromString, string RecipientString, string Body, string Subject)
        {
            if (String.IsNullOrEmpty(FromString)) throw new ArgumentNullException(nameof(FromString));
            if (String.IsNullOrEmpty(RecipientString)) throw new ArgumentNullException(nameof(RecipientString));
            if (String.IsNullOrEmpty(Body)) throw new ArgumentNullException(nameof(Body));
            if (String.IsNullOrEmpty(Subject)) throw new ArgumentNullException(nameof(Subject));

            List<MailAddress> recipients = TransformRecipients(RecipientString);
            MailAddress fromAddress = TransformOneRecepient(FromString);
            return SendInternal(recipients, fromAddress, Subject, Body);
        }

The only private field is SmtpClient now. Which is needed to send emails, and then the EmailSender object, once initialized, can be used multiple times to send multiple emails. SendInternal now requires four parameters to run, which means that TransformRecipient and TransformOneRecipient have to be run first. We just forced correct sequence. This is really useful in case you don’t want to use any complex state frameworks (to adopt class behavior to state diagram/flow). Our scenario of email sender class does not require such complexity. We can talk about stateful classes another time.

Thanks for reading

Dominik Steinhauf

CEO, IT for over 20 years, .Net developer, software architect at Creative Yellow Solutions (formerly Indesys), trainer and software development consultant for banking and energy sectors

If you need help with your software project, or need customized software for your company, contact me at:
dominik.steinhauf ( at) creativeyellow.pl