2016-11-05 29 views
4

我想重構下面的代碼,我該如何注入通知處理程序?最小的原始代碼更改和最佳重構(如有必要)。如何最好地重構此C#代碼?

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("[email protected]")); 

      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("[email protected]")); 

      smtpClient.Send(message); 
     } 
    } 
} 

我已經重構它像下面 -

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.", "[email protected]"); 
      } 
      } 
      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.", "[email protected]"); 
      } 
     } 
      else 
      { 
       throw new Exception("Id can not be null"); 
      } 
     } 
    } 
+2

你可能想合併'CloseMarket'和'OpenMarket'。您只能將其狀態更改爲關閉或打開。一個參數可以將2種方法減少到1. – Badiparmagi

+0

我會與Badipamagi的建議一起使用。也請開始添加空檢查。 SendNotification中也可以使用相同的新參數。如果你沒有在其他地方使用發送通知,你可以合併它,但從測試點,並保持分離的擔憂,你可以保持原樣。 –

+0

也添加了null檢查,但它並不是一個好主意,因爲它對現在使用兩種方法的其他類和方法進行了巨大的重構,即「OpenMarket(Page page) and CloseMarket(Page page)' – Neo

回答

1

我會像這樣的東西去:

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.", "[email protected]"); 
      } 
     } 
     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.", "[email protected]"); 
      } 
     } 
     else 
     { 
      throw new Exception("entityId can not be null"); 
     } 
    }  
} 

在這是你的INotifier是違約d通常由構造函數實現,但可以在需要時使用訪問器覆蓋。如果你希望即使在不測試的時候總是注入你的依賴項,你也可以在構造函數中添加參數。

希望這會有所幫助。

+0

你可以編寫自己的代碼,而不需要複製粘貼人。 – khaled4vokalz

+0

公平點@ khaled4vokalz - 說實話,你的副本比OP更簡潔。當他們的_exactly_代碼相同時,我不想擁有兩個Open/CloseMarket方法副本。我會編輯立即刪除您的更改。 – s3raph86

+0

你可以保持它的兄弟....其好吧....) – khaled4vokalz

1

試試這個@ neo的

public class TestEventHandlers 
{ 
    public void OpenMarket(Page page) 
    { 
     ChangeMarketState(page, "Open", "[email protected]"); 
    } 

    public void CloseMarket(Page page) 
    { 
     ChangeMarketState(page, "Close", "[email protected]"); 
    } 

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