2015-04-20 14 views
6

Il refactoring è buono, ma a volte non è così semplice calcolare il refactoring e in effetti se qualcosa può essere effettivamente refactored!C# Ristrutturazione di due metodi quasi identici

Ho un numero di metodi che sono quasi identici - posso refactoring ma una parte del refactoring è oltre la mia logica.

Qui ci sono due metodi non-refactoring:

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

Con refactoring vorrei avere un metodo come questo che i metodi in precedenza non-refactoring chiamerebbero

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

Quindi quello che abbiamo quasi metodo completamente refactored - con un problema, Come posso sapere quale form desidero istanziare dalla variabile frmName?

+1

Cosa si aspetta accada a DockState.Dock *? Il metodo refactored sarà solo DockState.DockRight? O dovrebbe cambiare a seconda del tipo che stai usando? – Cubia

+0

@Cubia ti ha segnalato per averlo indicato. Anche se ho notato l'errore non appena ho testato la risposta: P –

risposta

8

È possibile rendere il metodo generico e sfruttare il vincolo generico 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; 
} 

Così si potrebbe chiamare come

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

Una limitazione è che il modulo dovrebbe avere un costruttore senza parametri pubblica. Se si dispone di un Form con costruttore parametrizzato, è possibile passare Func<TForm> al metodo che funge da metodo di fabbrica.

+0

frmName.Show() ha due diversi parametri in ogni metodo. Come faresti funzionare? – Cubia

+1

A causa del compito 'frmName' dovrebbe essere di riferimento o di ritorno. – Alex

+0

@Alex Non mi piace 'ref' aggiornerò il mio post per utilizzare il valore restituito. Grazie. –

1

passare una fabbrica per il metodo, in questo modo:

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

L'unico requisito è che entrambe le classi create bisogno di avere un po 'di interfaccia comune o classe di base.

1

Vedo che c'è già una risposta, ma ancora voglia di scrivere qualcosa di più. Credo che il refactoring sia molto più di quello che descrivi qui. Una cosa sta cambiando i nomi per la funzione, uno sta mettendo il codice in una funzione separata. Ricorda inoltre che non esiste un refactoring adeguato senza i test di unità appropriati.

Nel vostro esempio si stanno mixando funzioni di alto livello con funzioni di basso livello (cambiando il falso in vero, creando oggetti ecc.), Anche la funzione stessa non viene descritta da sé.

Quindi non c'è molto da fare in termini di refactoring:

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); 
} 
Problemi correlati