7
votes

C # refactoring deux méthodes presque identiques

Le refactoring est bon, mais il n'est parfois pas si facile de déterminer comment refactoriser et bien que quelque chose puisse réellement être refactored!

J'ai un certain nombre de méthodes presque identiques - je peux les refracter mais une partie du refactoring est au-delà de ma logique. P>

Voici deux méthodes non refacturées: P>

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


2 commentaires

Qu'attendez-vous d'arriver à dockstate.dock *? La méthode refactable sera-t-elle seulement une solution de dockstate.dockright? Ou devrait-il changer en fonction du type que vous utilisez?


@Cubia vous a marqué pour le pointer de la sorte. Bien que j'ai remarqué l'erreur dès que j'ai testé la réponse: p


3 Réponses :


1
votes

Passez une usine à la méthode, comme ceci:

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


0 commentaires

8
votes

Vous pouvez faire la méthode générique et tirer parti de nouvelle () code> contrainte générique.

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


6 commentaires

frmname.show () a deux paramètres différents dans chaque méthode. Comment feriez-vous ce travail?


En raison de la mission frmname devrait être de la référence ou du retour.


@Alex je n'aime pas ref Je mettrai à jour mon message pour utiliser la valeur de retour. Merci.


Dans ce cas, je pense que le dockstate doit également être transmis comme un paramètre


@Cubia Vous voulez dire que différents arguments sont passés? Si oui, prenez aussi un paramètre aussi. Si vous obtenez trop de paramètres, créez une classe et faites-leur un champ d'instance.


@Deduardowada vous avez raison. Mais ajoutant plus de deux paramètres à une méthode peut devenir compliqué, je recommanderais de créer une classe pour cela seul. et laissez quelque chose à être le champ de classe.



1
votes

Je vois qu'il y a déjà de réponse pourtant vouloir écrire quelque chose de plus. Je crois que le refactoring est beaucoup plus que ce que vous décrivez ici. Une chose change de noms pour la fonction, on met le code dans une fonction distincte. N'oubliez pas non plus qu'il n'y a pas de refactoring approprié sans tests d'unités appropriés.

Dans votre exemple, vous mélangez des fonctions de haut niveau avec des fonctions de faible niveau (Changer de faux à True, à la création d'objets, etc.), fonctionne également n'est pas décrit auto. P>

Il y a donc beaucoup à faire en termes de refactoring: P>

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


0 commentaires