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
}
}