What is the best way to reorganize this code in C #?

Question

I want to reorganize the following code, how do I add a notification handler? And minimal initial code changes and better refactoring where necessary.

public class TestEventHandlers
{
    public TestEventHandlers() { }

    public void OpenMarket(Page page)
    {
        var Id = page.Request["MarketId"];

        var repository = new EntityRepository();
        IEntity market = repository.GetById(Id);

        if (market.State != "Open")
        {
            throw new Exception("The market is not open!");
        }
        else
        {
            market.Open();

            repository.SaveChangesTo(market);

            var smtpClient = new SmtpClient();

            var message = new MailMessage();
            message.Subject = "market open";
            message.Body = market.ToString() + " was open.";
            message.To.Add(new MailAddress("[e-mail]"));

            smtpClient.Send(message);
        }
    }

    public void CloseMarket(Page page)
    {
        var Id = page.Request["MarketId"];
        var repository = new EntityRepository();
        IEntity market = repository.GetById(Id);

        if (market.State == "Close")
        {
            throw new Exception("The market is already close!");
        }
        else
        {
            market.Close();

            repository.SaveChangesTo(market);

            var smtpClient = new SmtpClient();

            var message = new MailMessage();
            message.Subject = "market closed";
            message.Body = market.ToString() + " has been closed.";
            message.To.Add(new MailAddress("[e-mail]"));

            smtpClient.Send(message);
        }
    }
}

I have already edited it as below -

public class TestEventHandlers
{
        public TestEventHandlers() { }

        public void OpenMarket(Page page)
        {
            var Id = page.Request["MarketId"];
            if (id!=null)
            { 
            var repository = new EntityRepository();
            IEntity market = repository.GetById(Id);

            if (market.State != "Open")
            {
                throw new Exception("The market is not open!");
            }
            else
            {
                market.Open();

                repository.SaveChangesTo(market);

                SendEmailNotification("market open", market.ToString() + " was open.", "[e-mail]");
            }
           }
          else
            {
                throw new Exception("Id can not be null");
            }
        }

        private static void SendEmailNotification(string subject,string body,string emailAddress)
        {
            var smtpClient = new SmtpClient();

            var message = new MailMessage();

            message.Subject = subject;
            message.Body = body;
            message.To.Add(new MailAddress(emailAddress));

            smtpClient.Send(message);
        }

        public void CloseMarket(Page page)
        {
            var Id = page.Request["MarketId"];
            if(id!=null)
            {
            var repository = new EntityRepository();
            IEntity market = repository.GetById(Id);

            if (market.State == "Close")
            {
                throw new Exception("The market is already close!");
            }
            else
            {
                market.Close();

                repository.SaveChangesTo(market);

                SendEmailNotification("market closed", market.ToString() + " has been closed.", "[e-mail]");
            }
        }
           else
            {
                throw new Exception("Id can not be null");
            }
       }
    }

Answer 1

I would say something like this:

public interface INotifier
{
    void SendEmailNotification(string subject, string body, string emailAddress);
}

public class Notifier : INotifier
{
    public void SendEmailNotification(string subject,string body,string emailAddress)
    {
        var smtpClient = new SmtpClient();

        var message = new MailMessage();

        message.Subject = subject;
        message.Body = body;
        message.To.Add(new MailAddress(emailAddress));

        smtpClient.Send(message);
    }
}

public class TestEventHandlers
{       
    public INotifier Notifier { get; set; }

    public TestEventHandlers()
    {           
        Notifier = new Notifier();
    }

    public void OpenMarket(Page page)
    {
        var Id = page.Request["MarketId"];
        if (id!=null)
        { 
            var repository = new EntityRepository();
            IEntity market = repository.GetById(Id);

            if (market.State != "Open")
            {
                throw new Exception("The market is not open!");
            }
            else
            {
                market.Open();

                repository.SaveChangesTo(market);

                Notifier.SendEmailNotification("market open", market.ToString() + " was open.", "[e-mail]");
            }
        }
        else
        {
            throw new Exception("entityId can not be null");
        }           
    }

    public void CloseMarket(Page page)
    {
        var Id = page.Request["MarketId"];
        if(id!=null)
        {
            var repository = new EntityRepository();
            IEntity market = repository.GetById(Id);

            if (market.State == "Close")
            {
                throw new Exception("The market is already close!");
            }
            else
            {
                market.Close();
                repository.SaveChangesTo(market);
                Notifier.SendEmailNotification("market closed", market.ToString() + " has been closed.", "[e-mail]");
            }
        }
        else
        {
            throw new Exception("entityId can not be null");
        }
    }       
}

In this, your INotifierdefault is used by your normal implementation by the constructor, but can be redefined if necessary using accessories. If you prefer to always introduce your dependencies, even if you are not testing, you can simply add arguments to the constructor.

Hope this helps.

Answer 2

Try @Neo

public class TestEventHandlers
{
    public void OpenMarket(Page page)
    {
        ChangeMarketState(page, "Open", "[e-mail]");
    }

    public void CloseMarket(Page page)
    {
        ChangeMarketState(page, "Close", "[e-mail]");
    }

    private static void SendEmailNotification(string subject,string body,string emailAddress)
    {
        var smtpClient = new SmtpClient();

        var message = new MailMessage();

        message.Subject = subject;
        message.Body = body;
        message.To.Add(new MailAddress(emailAddress));

        smtpClient.Send(message);
    }

    public void ChangeMarketState(Page page, string changeStateTo, string sendMailTo)
    {
        var Id = page.Request["MarketId"];
        if(Id != null)
        {
            var repository = new EntityRepository();
            IEntity market = repository.GetById(Id);

            if (market.state == changeStateTo)
            {
                if(changeStateTo == "Close")
                    throw new Exception("The market is already close!");
                else
                    throw new Exception("The market is not open!");
            }
            else
            {
                string currentMarketState = string.empty;
                string mailHeader = string.empty;
                if(changeStateTo == "Close")
                {
                   market.Close();
                   currentMarketState = market.ToString() + " has been closed.";
                   mailHeader = "market closed";
                }
                else
                {
                   market.Open();
                   currentMarketState = market.ToString() + " was open.";
                   mailHeader = "market open";
                }

                repository.SaveChangesTo(market);

                SendEmailNotification(mailHeader, currentMarketState, sendMailTo);
            }
        }
        else
        {
            throw new Exception("entityId can not be null");  
        }
    }
}