2015-04-20 6 views
6

Refaktoryzacja jest dobra, ale czasami nie jest łatwo obliczyć, jak refaktoryzować, a nawet czy coś rzeczywiście może być refaktoryzowane!C# Refaktoryzacja dwiema prawie identycznymi metodami

Mam wiele metod, które są prawie identyczne - mogę je zreorganizować, ale jedna część refaktoryzacji wykracza poza moją logikę.

Oto dwa un-refactored metody:

private void projectToolStripMenuItem_Click(object sender, EventArgs e) 
    { 
     if (projectToolStripMenuItem.Checked) 
     { 
      projectToolStripMenuItem.Checked = false; 
      if (!projectForm.IsDisposed) projectForm.Hide(); 
     } 
     else 
     { 
      if (projectForm.IsDisposed) 
       projectForm = new frmProject(); 
      projectForm.Show(dockPanel, DockState.DockRight); 
      projectToolStripMenuItem.Checked = true; 
     } 

    } 

    private void logginToolStripMenuItem_Click(object sender, EventArgs e) 
    { 
     if (logginToolStripMenuItem.Checked) 
     { 
      logginToolStripMenuItem.Checked = false; 
      if (!outputForm.IsDisposed) outputForm.Hide(); 
     } 
     else 
     { 
      if (outputForm.IsDisposed) 
       outputForm = new frmOutput(); 
      outputForm.Show(dockPanel, DockState.DockBottom); 
      logginToolStripMenuItem.Checked = true; 
     } 
    } 

Z Refaktoryzacja chciałbym uzyskać metodę tak, że wcześniej un-refactored metody nazwałbym

private void refactoredMethod(TooStripMenuItem menuItem, DockContent frmName) 
{ 

     if (menuItem.Checked) 
     { 
      menuItem.Checked = false; 
      if (!frmName.IsDisposed) frmName.Hide(); 
     } 
     else 
     { 
      if (frmName.IsDisposed) 
       frmName= new frmProject(); // Still Problematic 
      frmName.Show(dockPanel, DockState.DockRight); 
      menuItem.Checked = true; 
     } 
    } 

Więc co mamy prawie metoda całkowicie refaktoryzowana - z jednym problemem, Jak mogę określić, który form chcę utworzyć z zmiennej frmName?

+1

Czego spodziewa się zdarzyć DockState.Dock *? Czy metodą refaktoryzacji będzie tylko DockState.DockRight? A może powinien się zmienić w zależności od tego, jakiego typu używasz? – Cubia

+0

@Cubia zaznaczyła cię za wskazanie tego. Chociaż zauważyłem błąd, gdy tylko przetestowałem odpowiedź: P –

Odpowiedz

8

Można uczynić metodę ogólną i wykorzystać ogólne ograniczenie new().

private TForm refactoredMethod<TForm>(TooStripMenuItem menuItem, TForm frmName) where TForm : Form, new() 
{ 
    if (menuItem.Checked) 
    { 
     menuItem.Checked = false; 
     if (!frmName.IsDisposed) frmName.Hide(); 
    } 
    else 
    { 
     if (frmName.IsDisposed) 
      frmName= new TForm(); 
     frmName.Show(dockPanel, DockState.DockRight); 
     menuItem.Checked = true; 
    } 
    return frmName; 
} 

Więc można nazwać jako

projectForm = refactoredMethod<frmProject>(projectToolStripMenuItem, projectForm); 

Jedynym ograniczeniem jest to, że forma powinna mieć publicznego konstruktora bez parametrów. Jeśli masz Form z parametryzowanym konstruktorem, możesz przekazać Func<TForm> do swojej metody, która działa jak metoda fabryczna.

+0

frmName.Show() ma dwa różne parametry w każdej metodzie. Jak byś to zrobił? – Cubia

+1

Z powodu przypisania 'frmName' powinno być przez ref lub zwracane. – Alex

+0

@Alex Nie podoba mi się 'ref' Zaktualizuję mój post, aby użyć wartości zwracanej. Dzięki. –

1

zdać fabrykę do sposobu, jak to:

private void RefactoredMethod(..., Func<TypeOfItemToCreate> creator) 
{ 
    ... 
    if (frmName.IsDisposed) 
      frmName = creator(); 
} 

Jedynym wymogiem jest to, że zarówno z klas tworzonych trzeba mieć jakiś wspólny interfejs lub klasę bazową.

1

Widzę, że istnieje już odpowiedź, ale chcę napisać coś więcej. Uważam, że refaktoryzacja to znacznie więcej niż opisujesz tutaj. Jedną z rzeczy jest zmiana nazwy funkcji, jedna polega na umieszczeniu kodu w oddzielnej funkcji. Pamiętaj również, że nie ma właściwego refaktoryzacji bez odpowiednich testów jednostkowych.

W twoim przykładzie mieszamy funkcje wysokiego poziomu z funkcjami niskiego poziomu (zmieniając na true wartość false, tworząc obiekty itp.), Sama funkcja nie jest opisana samodzielnie.

Więc nie ma wiele do zrobienia w zakresie refaktoringu:

void hideForm(TForm form){ 
    if(!form.IsDisposed){ 
     form.Hide(); 
    } 
} 

void showFormInDockPanel<TForm>(TForm form, DockPanel dockPanel){ 
    if(form.IsDisposed){ 
     form = new TForm(); 
    } 
    form.Show(dockPanel, DockState.DockRight); 
} 

void toggleFormAndMenuItem<TForm>(TForm form, TooStripMenuItem menuItem){ 
    if(menuItem.checked){ 
     hideForm(form); 
    } 
    else { 
     showFormInDockPanel<TForm>(form, dockPanel); 
    } 

    menuItem.checked = !menuItem.checked; 
} 

private void projectToolStripMenuItem_Click(object sender, EventArgs e){ 
    toggleFormAndMenuItem<frmProject>(projectToolStripMenuItem, projectForm); 
}