Code refactoring c# Frage um mehrere if-Anweisungen, die die gleiche Prüfung durchführen

Question

Ich habe eine Methode, die eine Menge von if-Anweisungen, die ein bisschen dumm scheint, obwohl ich nicht sicher bin, wie der Code zu verbessern.

Hier ist ein Beispiel. Diese Logik war in der Ansicht, die jetzt in der Steuerung ist, die weit besser ist, aber gibt es etwas, das ich vermisse, vielleicht ein Design-Muster, das mich stoppt, gegen panelCount < NumberOfPanelsToShow und Handhabung der panelCount jede Bedingung zu überprüfen? Vielleicht nicht, fühlt sich einfach hässlich an!

Vielen Dank!

if (model.Train && panelCount < NumberOfPanelsToShow)
{
    panelTypeList.Add(TheType.Train);
    panelCount++;
}
if (model.Car && panelCount < NumberOfPanelsToShow)
{
    panelTypeList.Add(TheType.Car);
    panelCount++;
}
if (model.Hotel && panelCount < NumberOfPanelsToShow)
{
    panelTypeList.Add(TheType.Hotel);
    panelCount++;
}
...

Answer 1

Unter der Annahme, dass Model.Train, Model.Car, Model.Plane nur boolesche Indikatoren für den Typ des "Modells" sind (anstatt dass Sie Train : Model, Plane : Model usw. erstellen)

public enum ModelType { Train, Car, Plane };

public class Model { 
    ... 
    public ModelType {get;set;}
    ... 
}

und in Ihrem Test:

if(panelCount < NumberOfPanelsToShow)
switch (Model.ModelType)
{
    case ModelType.Train :
        ...
        break;

    case ModelType.Plane :
...
}

JEDOCH, da Car, PLane und Train unterschiedlich sind, sollten Sie wirklich einen Basistyp Model haben, Car, Plane und Train von Model ableiten und dann können Sie Methoden überladen, um jeden Typ zu behandeln

if (panelCount < NumberOfPanelsToShow)
{
    panelCount += AddModel(model);
}

private int AddModel(Plane model)
{ // do plane stuff here and on success return 1 else 0; }

private int AddModel(Train model)
{ // do train stuff here and on success return 1 else 0; }

private int AddModel(Car model)
{ // do car stuff here and on success return 1 else 0; }

Answer 2

Normalerweise ist der Weg, dies zu refactor ist es polymorph mit der Verantwortung für die Entscheidung, was zu tun ist mit dem Objekt zu tun, aber wie Ihre Entscheidung auf verschiedene Aspekte des gleichen Objekts basiert, bin ich nicht sicher, dass Sie das hier tun können.

Ich wäre versucht, stattdessen in eine Methode umzuwandeln:

public AddIfSpace(bool thingToTest, TheType typeToAdd, ref currentCount)
{
    if (currentCount<NumberOfPanelsToShow && thingToTest)
    {
     panelTypeList.Add(typeToAdd);
     currentCount++;
    }
}

dann rufen Sie es an:

AddIfSpace(model.Car,TheType.Car,ref panelCount);
AddIfSpace(model.Train,TheType.train,ref panelCount);
AddIfSpace(model.Hotel,TheType.Hotel,ref panelCount);

benötigen Sie möglicherweise weniger oder mehr Parameter, je nach Umfang Ihrer Variablen, und Sie könnten es eine Erweiterungsmethode auf den Typ der panelTypeList machen, wenn das eingeschränkten Umfang hat, so dass Sie mit etwas wie enden:

panelTypeList.AddIfSpace(model.Car,TheType.Car,ref panelCount);
panelTypeList.AddIfSpace(model.Train,TheType.Train,ref panelCount);
panelTypeList.AddIfSpace(model.Hotel,TheType.Hotel,ref panelCount);

Dies vermeidet zumindest die wiederholte Logik in Ihrem Code und bedeutet, dass Sie eine einzige Stelle zum Ändern haben, wenn Sie die Funktionsweise der Prüfung ändern wollen, und wenn Sie die Methode schön benennen, sollte die Absicht deutlich werden und den Code lesbarer machen.

Answer 3

Ich werde einen Versuch wagen und einige Annahmen machen.

Es sieht so aus, als hätten Sie eine Art "Panel"-Klasse, die Sie mit Informationen aus Datenmodell-Objekten (Auto, Hotel, Zug...) zu füllen versuchen. Warum verwenden Sie stattdessen nicht eine MVVM Typmuster mit einer polymorphen Panel-Klasse (oder einer IPanel-Schnittstelle) und Unterklassen von CarPanel, HotelPanel, TrainPanel usw... Mit ViewModel-Objekten wird Ihr View-Code noch einfacher:

var newPanel = PanelFactory.GetPanel(data);  //Returns a CarPanel, HotelPanel, TrainPanel...
panelTypeList.Add(newPanel);
panelCount++;

Answer 4

Ich bin mir nicht sicher, wie man dieses Entwurfsmuster nennen könnte. Aber ich habe einmal gelesen, dass man, wenn man immer wiederkehrende Codeblöcke für verschiedene Typen macht (wie Sie es gerade tun), diese Funktionalität besser in die Objekte selbst .

Sie können es so aufschlüsseln:

private const NumberOfPanelsToShow = 4;
private int panelCount = 0;

private void addToPanel(IPanelAddable element, PanelList panelTypeList) {
  if(panelCount < NumberofPanelsToShow) {
    element.addToPanelTypeList(panelTypeList);
    panelCount++;
  }
}

IPanelAddable kann eine Schnittstelle sein, die die Methode addToPanelTypeList (oder was auch immer) definiert. Dann in Ihrer ursprünglichen Methode:

if (model.Train)
{
    addToPanel(TheType.Train);
}
if (model.Car)
{
    addToPanel(TheType.Car);
}
if (model.Hotel)
{
    addToPanel(TheType.Hotel);
}

Eigentlich wollen Sie das oben genannte vermeiden, vereinfachen Sie die oben /w eine Basisklasse, etc.... Dann müssten Ihre TheType.*-Objekte natürlich die Methode "addToPanelTypeList" implementieren:

panelTypeList.Add(this);

Sie können Schnittstellen oder Vererbung verwenden, um das gleiche Ziel zu erreichen...

Answer 5

Was ich wahrscheinlich tun würde, ist ein Enum-Typ auf dem Modell zu wechseln gegen.

if(panelCount < NumberOfPanelsToShow)
{
  switch(model.modelType)
  {
   case Model.Car:
    // do stuff
  break;
  }
  panelCount++;
}

Wir benötigen jedoch mehr Informationen, um zu bestimmen, wie die Fälle zu behandeln sind und wie viele Bedingungen zu einem bestimmten Zeitpunkt gelten können.

UPDATE

Angesichts der zusätzlichen Informationen, die Sie geliefert haben, scheinen einige der anderen Antworten hier angemessener zu sein. Diese Antwort wird in Ihrem aktuellen Fall nicht ausreichen.