Real world refactoring

I often find code like this in some projects:

      public bool ShowDialog(object view, string title)   
      {   
		  //some code   
  
          var window = GetWindow(control,title);   
  
		  //some code   
  
          using (container)   
              return (bool) window.ShowDialog();   
      }   
  
		/////////////////////////////////////////   
		// a lot code between   
		/////////////////////////////////////////   
  
      public void Show(Control view, string title)   
      {   
    	  //some code   
  
          var window = GetWindow(view, window);   
  
    	  //some code   
  
          window.Show();   
      }   
  
		/////////////////////////////////////////   
		// a lot code between   
		/////////////////////////////////////////   
  
      private Window GetWindow(Control control, string title)   
      {   
            var window = new Container(control);   
  
			//doings something else with container, like:   
           
        	window.Text = title;   
  
	        return window;   
      }  

I this case Show and ShowDialog have really different logic and not well suitable for functional-style as I think, that’s not the case.

The case is method GetWindow itself – it is like situation when solving one problem you creates another.

As I think developer has following code:

      public bool ShowDialog(object view, string title)   
      {   
		  //some code   
  
		  var window = new Container(control);   
  
	 	  //doings something else with container, like:   
           
          window.Text = title;
  
		  //some code   
  
          using (container)   
              return (bool) window.ShowDialog();   
      }   
  
		/////////////////////////////////////////   
		// a lot code between   
		/////////////////////////////////////////   
  
      public void Show(Control view, string title)   
      {   
    	  //some code   

		  var window = new Container(control);   
  
	 	  //doings something else with container, like:   
           
          window.Text = title;   
   
    	  //some code   
  
          window.Show();   
      }   
 

His intention was to remove code duplication and it’s right, but… But He prefer to “Extract method” GetWindow, which give us code that we have. Yes, this is one way to do, but it’s not best, because in this case when somebody wants to read or edit this code it’s a very annoying. You need to switch your view between such “Top-level” public method and that “Extracted” private methods, it’s in such big class, especially bad when you have several methods like GetWindow. You just violates SRP.

You can avoid it really easy – you have 2 alternatives:

First

Just overload constructor of Window and implement all you need there:

      public bool ShowDialog(object view, string title)   
      {   
		  //some code   
  
          var window = new Container(control,title);   
  
		  //some code   
  
          using (container)   
              return (bool) window.ShowDialog();   
      }   
  
		/////////////////////////////////////////   
		// a lot code between   
		/////////////////////////////////////////   
  
      public void Show(Control view, string title)   
      {   
    	  //some code   
  
          var window = new Container(view, window);   
  
    	  //some code   
  
          window.Show();   
      }   
  
//Container.cs:

      public class Container
      {   
			public class Container(Control control, string title, /*any other params you need to set it properly */)
			{
				//doings something you need
           
        		window.Text = title;   
  
				//doings something you need
			}
      }  

Second

The bad thing of first way is that 1) You can have no access to/can’t inherit from class Container 2) You can hides some logic in Container which not natural to class Container. If one of these, or both – create Factory:

	  IContainerFactory containerFactory; 

      public bool ShowDialog(object view, string title)   
      {   
		  //some code   
  
          var window = containerFactory.Get(control,title);   
  
		  //some code   
  
          using (container)   
              return (bool) window.ShowDialog();   
      }   
  
		/////////////////////////////////////////   
		// a lot code between   
		/////////////////////////////////////////   
  
      public void Show(Control view, string title)   
      {   
    	  //some code   
  
          var window = containerFactory.Get(view, window);   
  
    	  //some code   
  
          window.Show();   
      }   
  
// ContainerFactory.cs:

      public class ContainerFactory
      {   
			public Get(Control control, string title, /*any other params you need to set it properly */)
			{

				//doings something you need
           
	        	window.Text = title;   
  
				//doings something you need

			}
      }