# Is it possible to avoid enormously big switch in that case? [duplicate]

I'm writing a simple chess-related code with intention to write it clearly (performance doesn't matter at all). And this method I have doesn't look clean to me at all:

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

I store the pieces as list, so, here I'm using LINQ to find the piece closest to a given square on a given direction.

1. Do switch/if statements of this size (8 cases total here) always present a code smell?
2. Is there any way to refactor this part only, or should I consider reviewing the design of the whole solution?
-

## marked as duplicate by DKnight, Walter, gnat, MichaelT, ChrisF♦Feb 13 '13 at 21:09

Consider adding polymorphism to Direction. `return direction.doStuff(squareFrom);` – user40980 Feb 13 '13 at 19:09
I wouldn't say that a moderately sized switch statement is always a code smell, especially when there is a natural limit to the number of cases (i.e., there are only ever 8 surrounding squares). – mcknz Feb 13 '13 at 19:14
Both portions of this question are on-topic. The question about code smells is certainly appropriate for P.SE. The refactoring portion of the question could be written better to fit more easily within P.SE, but it's on-topic as well. – GlenH7 Feb 13 '13 at 20:35
Now that this has been marked as a duplicate, should it be migrated to CodeReview.SE so that the OP can get an answer? The question cited as a duplicate presents an abstract case, where this is a specific instance. – mcknz Feb 13 '13 at 22:23

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

The size of the switch doesn't particularly matter, the number of times you have the switch does. If you are repeating yourself with this switch, it could or should indicate you need a better design perhaps using polymorphism. If your switch lives here and only here, then you might just live with it.

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

See point #1 about when to certainly consider a different approach. As for more immediate refactorings, at least consider extracting the bodies of the "case" code to delegate elsewhere.

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

Keep your one method concerned only with the switch, have other methods concerned with the appropriate filtering and ordering.

-
Also a good reading regarding the "polymorphism" part: sourcemaking.com/refactoring/… – heltonbiker Jul 25 '14 at 14:39

Have a look at the typesafe enum pattern and the command pattern. Don't use a C# enum in that case, but an interface.

``````interface Direction {

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

}
``````

So you just need to do:

``````Directions.ValueOf(someDirectionLiteral).Closest(_pieces)
``````

The `Directions.ValueOf` call maps a literal (a string or whatever) to an instance of the `Direction` interface.

The code may not be valid C#, but you should get.

-