C#•9mo ago

How to refactor lots of overloaded methods more smartly?

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

namespace MethodOverload
{
    internal class Program
    {
        static void Main(string[] args)
        {
            var person1 = new PersonModel("Peter", "Wow","[email protected]");
            person1.GenerateEmail("gmail.com", true);
            Console.WriteLine(person1.Email);
            Console.ReadLine();
        } 
    }

    public class PersonModel
    {
        public string FirstName { get; set; }
        public string LastName { get; set; }
        public string Email { get; set; }
        public PersonModel()
        {
            //method overloading
            
        }
        public PersonModel(string firstName, string lastName, string email)
        {
            FirstName = firstName;
            LastName = lastName;
            Email = email;
        }

        public void GenerateEmail()
        {
            Email = $"{FirstName}.{LastName}@aol.com";
        }
        public void GenerateEmail(string domain)
        {
            Email = $"{FirstName}.{LastName}@{domain}";
        }

        public void GenerateEmail(bool firstInitialMethod)
        {
            if (firstInitialMethod == true)
            {
                Email = $"{FirstName.Substring(0,1)}{LastName}@aol.com";
            }
            else
            {
                Email = $"{FirstName}.{LastName}@aol.com";
            }
        }

        public void GenerateEmail(string domain, bool firstInitialMethod)
        {
            if (firstInitialMethod == true)
            {
                Email = $"{FirstName.Substring(0, 1)}{LastName}@{domain}";
            }
            else
            {
                Email = $"{FirstName}.{LastName}@{domain}";
            }
        }
    }
}

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

namespace MethodOverload
{
    internal class Program
    {
        static void Main(string[] args)
        {
            var person1 = new PersonModel("Peter", "Wow","[email protected]");
            person1.GenerateEmail("gmail.com", true);
            Console.WriteLine(person1.Email);
            Console.ReadLine();
        } 
    }

    public class PersonModel
    {
        public string FirstName { get; set; }
        public string LastName { get; set; }
        public string Email { get; set; }
        public PersonModel()
        {
            //method overloading
            
        }
        public PersonModel(string firstName, string lastName, string email)
        {
            FirstName = firstName;
            LastName = lastName;
            Email = email;
        }

        public void GenerateEmail()
        {
            Email = $"{FirstName}.{LastName}@aol.com";
        }
        public void GenerateEmail(string domain)
        {
            Email = $"{FirstName}.{LastName}@{domain}";
        }

        public void GenerateEmail(bool firstInitialMethod)
        {
            if (firstInitialMethod == true)
            {
                Email = $"{FirstName.Substring(0,1)}{LastName}@aol.com";
            }
            else
            {
                Email = $"{FirstName}.{LastName}@aol.com";
            }
        }

        public void GenerateEmail(string domain, bool firstInitialMethod)
        {
            if (firstInitialMethod == true)
            {
                Email = $"{FirstName.Substring(0, 1)}{LastName}@{domain}";
            }
            else
            {
                Email = $"{FirstName}.{LastName}@{domain}";
            }
        }
    }
}

3 Replies

SteadhavenOP•9mo ago

I was curious if there was a smarter way of handling the GenerateEmail than what I did? Its a learning example to learn method overloading, but it seems rather repeating (not DRY). Is there a way to refactor it and make it more concise? (its all the GenerateEmail() method overloads)

Keswiik•9mo ago

First thought after looking at this is that email generation logic should not be in your model. Either way, this looks like it'd be a good candidate for optional method parameters. I'd also suggest not using a bool to flip between your generation modes as you're stuck with only two options and an enum would work better instead. For example:

public enum EmailGenerationMode {
  FullName,
  FirstInitial,
  ...
}

public string GenerateEmail(EmailGenerationMode mode = EmailGenerationMode.FullName, string domain = "aol.com") {
  return mode switch {
    EmailGenerationMode.FullName => <your logic here>,
    EmailGenerationMode.FirstInitial => <your logic here>
  };
}

public enum EmailGenerationMode {
  FullName,
  FirstInitial,
  ...
}

public string GenerateEmail(EmailGenerationMode mode = EmailGenerationMode.FullName, string domain = "aol.com") {
  return mode switch {
    EmailGenerationMode.FullName => <your logic here>,
    EmailGenerationMode.FirstInitial => <your logic here>
  };
}

lycian•9mo ago

another thing for overloading: often times overloads call the highest specificity with default values. For example from your initial code

c#
private const string DefaultDomain = "aol.com";
public void GenerateEmail() => GenerateEmail(DefaultDomain, false);
public void GenerateEmail(string domain) => GenerateEmail(domain, false);
public void GenerateEmail(bool firstInitialMethod) => GenerateEmail(DefaultDomain firstInitialMethod);
public void GenerateEmail(string domain, bool firstInitialMethod)
{
    if (firstInitialMethod == true)
    {
        Email = $"{FirstName.Substring(0, 1)}{LastName}@{domain}";
    }
    else
    {
        Email = $"{FirstName}.{LastName}@{domain}";
    }
}

c#
private const string DefaultDomain = "aol.com";
public void GenerateEmail() => GenerateEmail(DefaultDomain, false);
public void GenerateEmail(string domain) => GenerateEmail(domain, false);
public void GenerateEmail(bool firstInitialMethod) => GenerateEmail(DefaultDomain firstInitialMethod);
public void GenerateEmail(string domain, bool firstInitialMethod)
{
    if (firstInitialMethod == true)
    {
        Email = $"{FirstName.Substring(0, 1)}{LastName}@{domain}";
    }
    else
    {
        Email = $"{FirstName}.{LastName}@{domain}";
    }
}

but @ded did a great job of refactoring to show how optional parameters which is a better design for your current use case

Gaming

Programming

How to refactor lots of overloaded methods more smartly?