È possibile evitare un interruttore enormemente grande in quel caso? [duplicare]

Question

È possibile evitare un interruttore enormemente grande in quel caso? [duplicare]

#1 da (4 voti)
#2 da (1 voti)

4

Sto scrivendo un semplice codice relativo agli scacchi con l'intenzione di scriverlo chiaramente (le prestazioni non contano affatto). E questo metodo che ho non mi sembra affatto pulito:

 public static Piece GetClosestPiece(Square squareFrom, Direction direction)
    {
        //TODO: rework this mess
        switch (direction)
        {
            case Direction.Top:
                return
                    _pieces.Where(p => p.Square.OnVerticalWith(squareFrom))
                           .OrderBy(p => p.Square.Y)
                           .First(p => p.Square.Y > squareFrom.Y);
            case Direction.Bottom:
                return
                    _pieces.Where(p => p.Square.OnVerticalWith(squareFrom))
                           .OrderByDescending(p => p.Square.Y)
                           .First(p => p.Square.Y < squareFrom.Y);
            case Direction.Right:
            <...>
            case Direction.TopLeft:
                return
                    _pieces.Where(p => p.Square.OnBackslashWith(squareFrom))
                           .OrderByDescending(p => p.Square.X)
                           .First(p => p.Square.X < squareFrom.X);
            default:
                throw new InvalidEnumArgumentException();
        }
    }

Conservo i pezzi come lista, quindi, qui sto usando LINQ per trovare il pezzo più vicino a un dato quadrato in una determinata direzione.

Le frasi switch / if di queste dimensioni (qui 8 casi totali) presentano sempre un odore di codice?
C'è un modo per refactare questa parte solo, o dovrei prendere in considerazione la revisione della progettazione dell'intera soluzione?

c# refactoring switch-statement enum

posta vorou 13.02.2013 - 20:06

fonte

2 risposte

1

Dai un'occhiata al pattern enume tipografico e il schema di comando . Non utilizzare un enumerato C # in quel caso, ma un'interfaccia.

interface Direction {

    // implements a single case in your switch
    public Piece Closest(Collection<Piece> pieces);

}

Quindi devi solo fare:

Directions.ValueOf(someDirectionLiteral).Closest(_pieces)

La chiamata Directions.ValueOf mappa un valore letterale (una stringa o qualsiasi altra cosa) in un'istanza dell'interfaccia Direction .

Il codice potrebbe non essere valido C #, ma dovresti farlo.

risposta data 13.02.2013 - 20:36

fonte

Leggi altre domande sui tag c# refactoring switch-statement enum

È saggio includere qualcosa come OpenSSL o GnuTLS con un progetto in un repository? Come vengono modificati i Set Immutabili

score 4 · Accepted Answer

1. Do switch/if statements of this size (8 cases total here) always present a code smell?

La dimensione dell'interruttore non è particolarmente importante, il numero di volte che si ha l'interruttore. Se ti stai ripetendo con questo interruttore, potrebbe o dovrebbe indicare che hai bisogno di un design migliore, magari usando il polimorfismo. Se il tuo switch è presente qui e solo qui, potresti semplicemente conviverci.

2. Is there any way to refactor this part only, or should I consider reviewing the design of the whole solution?

Vedere il punto 1 su quando considerare sicuramente un approccio diverso. Per quanto riguarda i refactoring più immediati, considera almeno l'estrazione dei corpi del codice "case" da delegare altrove.

case Direction.Top:
     return GetFromTop(_pieces);
case Direction.Bottom:
     return ...

Mantieni il tuo metodo in questione solo con lo switch, con altri metodi relativi al filtro e all'ordinamento appropriati.