Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is one of a classes (that sends email base on object type to AM (Role)) that is executed by a engine. I have also commands like SendEmailToAOCommand that send email to role AO base on object type and so one.

namespace SFE.Workflow.Commands
{
    public class SendEmailToAMCommand : ActionCommand
    {
        private readonly Func<IApplicationRepository> applicationRepository;
        private readonly IClientRepository clientRepository;
        private readonly Func<IEmailService> emailService;
        private readonly Func<IUserRepository> userRepository;


        public SendEmailToAMCommand(
            Func<IEmailService> _emailService,
            Func<IUserRepository> _userRepository,
            Func<IApplicationRepository> _applicationRepository,
            IClientRepository _clientRepository)
        {
            emailService = _emailService;
            CommandFor = Modules.All;
            userRepository = _userRepository;
            applicationRepository = _applicationRepository;
            clientRepository = _clientRepository;
        }

        public override string CommandDescription
        {
            get { return "Sends email to AM"; }
        }

        public override void Body(object _obj, object _objInPreviousState)
        {
            if (_obj == null)
                throw new ArgumentNullException("first param is null");

            string message = string.Empty;
            string subject = string.Empty;
            if (_objInPreviousState == null)
            {
                var emailParams = Param as Dictionary<string, string>;
                if (emailParams != null)
                {
                    message = emailParams["Message"];
                    subject = emailParams["Subject"];
                }
            }
            if (_obj is Application)
            {
                var app = (Application)_obj;
                var email = userRepository().GetManagerForUser(app.UserID).Email;
                Client borrower = clientRepository.GetMainBorrower(app.ID);
                if (_objInPreviousState == null)
                {
                    emailService().SendEmail("", email, message, subject);
                }
                else
                {
                    emailService().SendEmail("", email,"Application: " + borrower.CompanyName + " (" + app.ID +") changed decision status: " +Enum.GetName(typeof(AppStatus), app.ApplicationStatus),"Check following application: " + app.ID);
                }
            }
            else if (_obj is Product)
            {
                var product = (Product)_obj;
                var email = userRepository().GetManagerForUser(product.Application.UserID).Email;
                Client borrower = clientRepository.GetMainBorrower(product.ApplicationID);
                if (_objInPreviousState == null)
                {
                    emailService().SendEmail("", email, message, subject);
                }
                else
                {
                    emailService().SendEmail("", email,"Product: " + product.ID + " for application " + borrower.CompanyName +" (" + product.ApplicationID + ") changed decision status: " +Enum.GetName(typeof(AppStatus), product.ProductStatusType),"Check following application: " + product.ApplicationID);
                }
            }

            else if (_obj is CES)
            {
                var ces = (CES)_obj;
                User user = applicationRepository().GetByID(ces.ApplicationID).User;
                var email = userRepository().GetManagerForUser(user.UserName).Email;
                Client borrower = clientRepository.GetMainBorrower(ces.ApplicationID);
                if (_objInPreviousState == null)
                {
                    emailService().SendEmail("", email, message, subject);
                }
                else
                {
                    emailService().SendEmail("", user.Email,"CES for application " + borrower.CompanyName + " (" +ces.ApplicationID + ") changed decision status: " +Enum.GetName(typeof(CesStatuses), ces.Status),"Check following application: " + ces.ApplicationID);
                }
            }
            else if (_obj is Comment)
            {
                var comment = (Comment)_obj;
                Client borrower = clientRepository.GetMainBorrower(comment.ApplicationID);
                emailService().SendEmail("", comment.User.Email,"Comment for the following application: " + borrower.CompanyName + " (" +comment.ApplicationID + ") with message: " + comment.Message + " on date: " +comment.CreatedDate,"Comment for the following application: " + comment.ApplicationID);
            }
            else if (_obj is Memo)
            {
                var memo = (Memo)_obj;
                var email = userRepository().GetManagerForUser(memo.UserID).Email;
                Client borrower = clientRepository.GetMainBorrower(memo.ApplicationID);
                if (_objInPreviousState == null)
                {
                    emailService().SendEmail("", email, message, subject);
                }
                else
                {
                    emailService().SendEmail("", email, "Memo for application : " + borrower.CompanyName + " (" + memo.ApplicationID + ") changed decision status: " + Enum.GetName(typeof(AppStatus), memo.Status), "Check following memo for application: " + memo.ApplicationID);
                }
            }
            Logger.InfoLine("Sending Email done" + " @" + CommandName);

        }
    }
}

The problem: This pattern is used in all of other commands. How can I improve this so this will not have same code in over and over again?

Edited

Can you please review it and suggest improvements?

share|improve this question
1  
You want to modify your in a specific way, you're not asking us to review it and look for possible improvements. Because of that, I think your question doesn't fit on codereview well and would be better suited for SO. –  svick Dec 5 '12 at 10:49
    
@svick: Wouldn't SO have a tendency to close this as too localized? –  Brian Dec 6 '12 at 17:34
    
This doesn't seem to be about generics. Generics methods are basically like duck typing but in a statically defined manner -- you have an algorithm that does something, and instead of copy/pasting and the search and replace on the types, you use generics and end up with just one function just as you would if c# did duck typing. It is statically defined, so the type has to support all of the operations used (generally through interfaces but sometimes through inheritance and base types). –  jmoreno Dec 15 '13 at 9:02
add comment

1 Answer 1

up vote 1 down vote accepted

You don't provide any example of how SendEmailToAOCommand would differ from SendEmailToAMCommand, so I can't be more specific, but here's the gist of what you should do:

public abstract class SendEmailBase : ActionCommand
{
    private readonly Func<IApplicationRepository> applicationRepository;
    private readonly IClientRepository clientRepository;
    private readonly Func<IEmailService> emailService;
    private readonly Func<IUserRepository> userRepository;


    public SendEmailBase(
        Func<IEmailService> _emailService,
        Func<IUserRepository> _userRepository,
        Func<IApplicationRepository> _applicationRepository,
        IClientRepository _clientRepository)
    {
        emailService = _emailService;
        CommandFor = Modules.All;
        userRepository = _userRepository;
        applicationRepository = _applicationRepository;
        clientRepository = _clientRepository;
    }

    public override string CommandDescription
    {
        get { return "Sends email to " + EmailRecipient; }
    }
    public abstract string EmailRecipient { get; }

    public override void Body(object _obj, object _objInPreviousState)
    {
        if (_obj == null)
            throw new ArgumentNullException("first param is null");

        string message = string.Empty;
        string subject = string.Empty;
        if (_objInPreviousState == null)
        {
            var emailParams = Param as Dictionary<string, string>;
            if (emailParams != null)
            {
                message = emailParams["Message"];
                subject = emailParams["Subject"];
            }
        }
        if (_obj is Application)
        {
            var app = (Application)_obj;
            var email = userRepository().GetManagerForUser(app.UserID).Email;
            Client borrower = clientRepository.GetMainBorrower(app.ID);
            if (_objInPreviousState == null)
            {
                emailService().SendEmail("", email, message, subject);
            }
            else
            {
                emailService().SendEmail("", email,"Application: " + borrower.CompanyName + " (" + app.ID +") changed decision status: " +Enum.GetName(typeof(AppStatus), app.ApplicationStatus),"Check following application: " + app.ID);
            }
        }
        else if (_obj is Product)
        {
            var product = (Product)_obj;
            var email = userRepository().GetManagerForUser(product.Application.UserID).Email;
            Client borrower = clientRepository.GetMainBorrower(product.ApplicationID);
            if (_objInPreviousState == null)
            {
                emailService().SendEmail("", email, message, subject);
            }
            else
            {
                emailService().SendEmail("", email,"Product: " + product.ID + " for application " + borrower.CompanyName +" (" + product.ApplicationID + ") changed decision status: " +Enum.GetName(typeof(AppStatus), product.ProductStatusType),"Check following application: " + product.ApplicationID);
            }
        }

        else if (_obj is CES)
        {
            var ces = (CES)_obj;
            User user = applicationRepository().GetByID(ces.ApplicationID).User;
            var email = userRepository().GetManagerForUser(user.UserName).Email;
            Client borrower = clientRepository.GetMainBorrower(ces.ApplicationID);
            if (_objInPreviousState == null)
            {
                emailService().SendEmail("", email, message, subject);
            }
            else
            {
                emailService().SendEmail("", user.Email,"CES for application " + borrower.CompanyName + " (" +ces.ApplicationID + ") changed decision status: " +Enum.GetName(typeof(CesStatuses), ces.Status),"Check following application: " + ces.ApplicationID);
            }
        }
        else if (_obj is Comment)
        {
            var comment = (Comment)_obj;
            Client borrower = clientRepository.GetMainBorrower(comment.ApplicationID);
            emailService().SendEmail("", comment.User.Email,"Comment for the following application: " + borrower.CompanyName + " (" +comment.ApplicationID + ") with message: " + comment.Message + " on date: " +comment.CreatedDate,"Comment for the following application: " + comment.ApplicationID);
        }
        else if (_obj is Memo)
        {
            var memo = (Memo)_obj;
            var email = userRepository().GetManagerForUser(memo.UserID).Email;
            Client borrower = clientRepository.GetMainBorrower(memo.ApplicationID);
            if (_objInPreviousState == null)
            {
                emailService().SendEmail("", email, message, subject);
            }
            else
            {
                emailService().SendEmail("", email, "Memo for application : " + borrower.CompanyName + " (" + memo.ApplicationID + ") changed decision status: " + Enum.GetName(typeof(AppStatus), memo.Status), "Check following memo for application: " + memo.ApplicationID);
            }
        }
        Logger.InfoLine("Sending Email done" + " @" + CommandName);

    }
}

public class SendEmailToAMCommand : SendEmailBase
{
    public SendEmailToAMCommand(
             Func<IEmailService> _emailService,
             Func<IUserRepository> _userRepository,
             Func<IApplicationRepository> _applicationRepository,
             IClientRepository _clientRepository) : base(_emailService, _userRepository, _applicationRepository, _clientRepository) { }
    public override string EmailRecipient { get { return "AM"; } }
}

Everything that's the same gets put into SendEmailBase. Everything which could vary is defined as an abstract method or property, and then the subclasses implement just those things which make it different.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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