wake-up-neo.com

Wenn Zustand im Schaltergehäuse

Ich versuche, eine if-Anweisung in Fallwechsel zu konvertieren (zur besseren Lesbarkeit) 

1) Ich habe gelesen, dass switch-Anweisungen im Allgemeinen fürchterlich sind - Stimmt das? https://stackoverflow.com/questions/6097513/switch-statement-inside-a-switch-statement-c

2) Die Aussage lautet wie folgt:

switch (Show)
                {
                    case Display.Expense:
                        if (expected.EXPENSE != true)
                            break;
                    case Display.NonExpense:
                        if (expected.EXPENSE == true)
                            break;
                    case Display.All:
                        //Code
                        break;
                }

Fehler ist: 

Control kann nicht von einer Fallbezeichnung ('case 1:') zur anderen fallen

Dies ist die ursprüngliche if-Anweisung:

if ((Show == Display.All) || (expected.EXPENSE == true && Show == Display.Expense) || (expected.EXPENSE == false && Show == Display.NonExpense))
{
    //Code
}
9
AngelicCore

Der Compiler wird nicht verstehen, was Sie hier meinen.

switch (Show)
{
    case Display.Expense:
        if (expected.EXPENSE != true)
            break;
        // missing break here
    case Display.NonExpense:

Der Compiler verbindet die Punkte nicht und versteht, dass die break;-Anweisung in Ihrer if-Anweisung mit der switch-Anweisung verknüpft ist. Stattdessen wird versucht, es mit einer Schleife zu verknüpfen, da break;-Anweisungen allein mit Schleifen verwendet werden können, um sie zu durchbrechen.

Das bedeutet, dass Ihrem case-Block die break-Anweisung fehlt, um ihn abzuschließen, und der Compiler beschwert sich.

Anstatt zu versuchen, den notwendigen Code aus einer switch-Anweisung herauszuholen, würde ich stattdessen Ihre ursprüngliche if-Anweisung auflösen.

Das ist deins:

if ((Show == Display.All) || (expected.EXPENSE == true && Show == Display.Expense) || (expected.EXPENSE == false && Show == Display.NonExpense))
{
    //Code
}

So würde ich es schreiben:

bool doDisplayExpected =
       (Show == Display.All)
    || (Show == Display.Expense    && expected.EXPENSE)
    || (Show == Display.NonExpense && !expected.EXPENSE);
if (doDisplayExpected)
{
    // code
}

Sie haben nicht haben alles in eine Zeile zu packen.

Außerdem würde ich versuchen, die Eigenschaften so zu benennen, dass sie einfacher zu lesen sind. Ich würde die EXPENSE-Eigenschaft in IsExpense umbenennen, sodass der obige Code folgendermaßen lautet:

bool doDisplayExpected =
       (Show == Display.All)
    || (Show == Display.Expense    && expected.IsExpense)
    || (Show == Display.NonExpense && !expected.IsExpense);
if (doDisplayExpected)
{
    // code
}

Dann würde ich die Unterausdrücke idealerweise in Methoden umwandeln:

bool doDisplayExpected =
       ShowAll()
    || ShowExpense(expected)
    || ShowNonExpense(expected);
if (doDisplayExpected)
{
    // code
}

public bool ShowAll()
{
    return Show == Display.All;
}

public bool ShowExpense(Expected expected)
{
    return Show == Display.Expense && expected.EXPENSE;
}

public bool ShowNonExpense(Expected expected)
{
    return Show == Display.NonExpense && !expected.EXPENSE;
}

Dann können Sie den Ausdruck wieder in die if-Anweisung einfügen:

if (ShowAll() || ShowExpense(expected) || ShowNonExpense(expected))
{
    // code
}

Dies sollte einfacher zu lesen und später geändert werden.

Zunächst fällt mir auf, dass Sie vergessen haben, in Ihrem zweiten Punkt eine Frage zu stellen. Ich werde also einige Fragen an Sie richten, die sich auf Ihren zweiten Punkt beziehen:

Was bedeutet der Fehler "kann nicht durchfallen"?

Im Gegensatz zu C und C++ erlaubt C # kein versehentliches Durchfallen von einem Switch-Bereich zu einem anderen. Jeder Schaltabschnitt muss einen "unerreichbaren Endpunkt" haben. es sollte mit einer break, goto, return, throw oder (selten) unendlichen Schleife enden. 

Dies verhindert, dass der übliche Fehler versäumt wird, zu vergessen, dass er die Unterbrechung durchbricht und durchfällt.

Sie haben Ihren Code so geschrieben, als sei der Durchbruch legal. Ich vermute, dass Sie ein C-Programmierer sind.

Wie kann ich einen Durchbruch in C # erzwingen?

So was:

switch (Show)
{
case Display.Expense:
    if (expected.EXPENSE != true)
        break;
    else
        goto case Display.All;
case Display.NonExpense:
    if (expected.EXPENSE == true)
        break;
    else  
        goto case Display.All;
case Display.All:
    //Code
    break;
}

Nun kann der Erreichbarkeitsanalysator feststellen, dass der Endpunkt des Schaltabschnitts unabhängig vom Zweig des "Wenn" nicht erreicht werden kann.

Ist das ein guter Stil?

Ihr ursprünglicher Code war viel lesbarer.

Ich habe gelesen, dass switch-Anweisungen im Allgemeinen schrecklich sind - stimmt das? 

Meinungen variieren. Switch-Anweisungen sind sehr nützlich, wenn es eine kleine Anzahl sehr "knackiger" Alternativen gibt, deren Verhalten nicht auf komplexe Art und Weise interagieren. Einige Leute werden Ihnen sagen, dass Switched Logic stattdessen mit virtuellen Methoden oder Besuchermustern gehandhabt werden sollte, aber das kann auch missbraucht werden. 

Sollte ich in diesem speziellen Fall einen Schalter verwenden?

Ich würde nicht 

Wie würden Sie meinen Code verbessern?

if ((Show == Display.All) || 
    (expected.EXPENSE == true && Show == Display.Expense) || 
    (expected.EXPENSE == false && Show == Display.NonExpense))
{
    //Code
}

Benennen Sie die Dinge in C # nicht in ALLEN CAPS.

Zweitens, vergleichen Sie Booleans nicht mit wahr und falsch. Sie sind schon Booleaner! Wenn Sie die Wahrheit über Aussage X wissen wollen, würden Sie nicht auf Englisch sagen: "Ist es wahr, dass X wahr ist?" Sie würden sagen "Ist X wahr?" 

Ich würde wahrscheinlich schreiben:

if (Show == Display.All || 
    Show == Display.Expense && expected.Expense || 
    Show == Display.NonExpense && !expected.Expense)
{
    //Code
}

Oder noch besser, ich würde den Test in eine eigene Methode abstrahieren:

if (canDisplayExpenses())
{ 
    //Code
}

Oder abstrahieren Sie das Ganze weg:

DisplayExpenses();
12
Eric Lippert

Verwenden Sie if-Anweisungen und extrahieren Sie komplexe Bedingungen in Methoden, z

if (ShowAll() || ShowExpense())
{
}

Denken Sie immer an OOP und Polymorphismus, wenn Sie einen solchen "Schalter" schreiben, und fügen Sie einen weiterencase zu diesem Code hinzu, ein nightmare.

siehe this und ähnliche (C++) Anweisungen zum Konvertieren von Switches

PS Wenn Sie daran interessiert sind, Ihren Code clean und lesbar zu erstellen [], ziehen Sie in Betracht, Smalltalk Best Practice Patterns von Kent Beck und/oder Clean Code von Uncle Bob I wirklich genossen beide, sehr zu empfehlen.

5

Wenn Sie Readability wollen, werfen Sie einfach Ihren Syntax-Papierkorb weg:

if (Show == Display.All || expected.EXPENSE && Show == Display.Expense || !expected.EXPENSE && Show == Display.NonExpense)
{
    //Code
}
3
Dennis

Geben Sie den else-Teil für jeden von ihnen an, damit keine Fehler ausgegeben werden. Wie andere jedoch sagen, benötigen Sie in diesem Fall switch nicht.

switch (Show)
{
    case Display.Expense:
         if (expected.EXPENSE != true)
             // do what you want
             break;
         else 
             // do what you want
             break;
    case Display.NonExpense:
         if (expected.EXPENSE == true)
             // do what you want
             break;
         else 
             // do what you want
             break;
    case Display.All:
        //Code
        break;
}

Der Grund für diese Fehlermeldung ist, dass Sie keine break-Anweisungen definieren.

Sie haben die Variable break bedingt definiert. 

            switch (Show)
            {
                case Display.Expense:
                    if (expected.EXPENSE != true)
                        break;

                // Note that the break above is in scope of you if statement, and will
                // result in a compiler error
                case Display.NonExpense:
                    ...
            }

Stellen Sie sicher, dass jede case-Anweisung ihre eigene break hat, oder gruppieren Sie die case-Anweisungen wie folgt.

            switch (Show)
            {
                case Display.Expense:
                case Display.All:
                    // do stuff
                    // Expense and All have the same behavior
            }
1
bas

Ersetzen Sie die if-Anweisungen so, dass Sie sie folgendermaßen ausdrücken können:

if (isDisplayAll() || isExpense(expected) || isNonExpense(expected))
{
    // Code
}

Die extrahierte Logik:

private bool isDisplayAll()
{
    return (Show == Display.All);
}

private bool IsExpense(Expected expected)
{
    return expected.EXPENSE && (Show == Display.Expense);
}


private bool IsNonExpense(Expected expected)
{
    return !expected.EXPENSE && (Show == Display.NonExpense);
}
0
Matthew Watson

Stimmen Sie Dennis zu, Sie möchten für dieses Problem kein Schaltgehäuse.

Obwohl wahrscheinlich weniger lesbar, können Sie auch kürzere verwenden:

if (Show == Display.All || (expected.EXPENSE == (Show == Display.Expense)))
{
    //Code
}
0
publicgk