How this switch case has unreachable code?

Question

I was implementing some generic IEqualityComparer<T> Equal() method when the code in the switch is unreachable without visible reason for me:

public bool Equals(T x, T y)
{
    switch (nameof(T))
    {
        case nameof(Accessory):
            return (x as Accessory).Id == (y as Accessory).Id;//not reachable
        default:
            return false;
    }
}

Someone has a clue?

next time consider using a if/else instead of a switch case when you have 2 possible cases. You wouldn't have hit this issue :) — gmolaire, Aug 26 '18 at 03:01

Joe Sewell · Accepted Answer · 2018-08-26T00:45:48.173

8

nameof evaluates the name of the T at compile time, so it's a constant string, "T", and thus only the default case will ever be taken.

Here's an alternative implementation:

public bool Equals(T x, T y)
{
    if (x is Accessory && y is Accessory)
    {
        var ax = x as Accessory;
        var ay = y as Accessory;
        return ax.Id == ay.Id;
    }
    return false;
}

C# 7.1 introduces some syntactic sugar:

public bool Equals(T x, T y)
{
    if (x is Accessory ax && y is Accessory ay)
    {
        return ax.Id == ay.Id;
    }
    return false;
}

(Note that your excerpt returns false if both x and y are null; I haven't fixed this in my versions.)

edited Aug 26 '18 at 00:45

answered Aug 26 '18 at 00:09

Joe Sewell

6,067
1
21
34

Do you know any way to apply a generic approach that does work? – Eugene Aug 26 '18 at 00:14
2

If you have a constraint on T (perhaps an interface that Accessory and all other classes that you are going to use with this method), then you can use a switch with a "type pattern". Otherwise, convert the switch to an 'if' statement and use "typeof(T).Name" – Flydog57 Aug 26 '18 at 00:19
1

@Eugene I have updated the answer with examples of how to use type identity, rather than the name of the types, to distinguish what the parameters are. – Joe Sewell Aug 26 '18 at 00:47
@JoeSewell I found a better middle ground: `switch (typeof(T).Name) { case nameof(CustomClass): ...`
but still appreciate your posted solutions ;) – Eugene Aug 26 '18 at 06:58
1

"better" how? I'd avoid switching on names. What about `namespace EvilNamespace { class Accessory { ... } } ` Name would still return "Accessory" and would match your `nameof` expression. Better check the actual type with is/as since you're casting to it anyways – pinkfloydx33 Aug 26 '18 at 11:31

score 0 · Answer 2 · answered Aug 26 '18 at 01:07

Can use this. This checks for null x and y:

public bool Equals(T x, T y)
        {
            if (ReferenceEquals(x, y)) return true;
            if (ReferenceEquals(x, null)) return false;
            if (ReferenceEquals(y, null)) return false;


    if (x.GetType()

 != y.GetType()) return false;
            return x.Id == y.Id;
        }

How this switch case has unreachable code?

2 Answers2