Improving LINQ expression with inner method call and switch statement

I have the following class which returns an expression that then returns data. It seems a tad over complicated and I'm pretty sure LINQ has something to offer that makes my life much easier. But I simply cannot get it.

/// Provides methods and properties to query location details from a database.
internal sealed class LocationDatabaseDetailsRequest : DatabaseDetailsRequest
{
    /// 
    public override Func Resource
    {
        get
        {
            return
                c =>
                c.Locations.Select(
                    l =>
                    new LocationDataContract
                    {
                        Id = l.Id,
                        Coordinates = l.Coordinates,
                        Description = l.Description,
                        Name = l.Name,
                        Type = l.Type,
                        Details = this.GetDetails(l.Type, c)
                    }).SingleOrDefault(i => i.Id == this.Identifier);
        }
    }

    private LocationDetailsDataContract GetDetails(string detailsType, LocationContext locationContext)
    {
        switch (detailsType)
        {
            case "City":
                return locationContext.Set().SingleOrDefault(i => i.Id == this.Identifier);
            case "Planet":
                return locationContext.Set().SingleOrDefault(i => i.Id == this.Identifier);
            case "StarSystem":
                return locationContext.Set().SingleOrDefault(i => i.Id == this.Identifier);
            case "Sector":
                return locationContext.Set().SingleOrDefault(i => i.Id == this.Identifier);
            default:
                return null;
        }
    }
}

My problem is the method call. I know which sets the data context has, but they are fluid, meaning it is certain that new sets will get added.

Can I write this query shorter and well... better?

public override Func Resource
{
    get
    {
        return
            c =>
            c.Locations.Select(
                l =>
                new LocationDataContract
                {
                    Id = l.Id,
                    Coordinates = l.Coordinates,
                    Description = l.Description,
                    Name = l.Name,
                    Type = l.Type,
                    Details = this.GetDetails(l.Type, c)
                }).SingleOrDefault(i => i.Id == this.Identifier);
    }
}

This code is inefficient, because the body of your Select is being executed for every value in Locations. This is because SingleOrDefault checks every item in an enumerable for duplicates. Since you're only checking an id and the id is not changed in your Select perform your SingleOrDefault call first.

public override Func Resource
{
    get
    {
        return c =>
        {
            var location = c.Locations.SingleOrDefault(i => i.Id == this.Identifier);
            return location == null ? 
                null : 
                new LocationDataContract
                {
                    Id = location.Id,
                    Coordinates = location.Coordinates,
                    Description = location.Description,
                    Name = location.Name,
                    Type = location.Type,
                    Details = this.GetDetails(location.Type, c)
                });
        }
    }
}

This way, the LocationDataContract is only created if there is a single location in c that matches the identifier.

Structurally, I'm wondering why you have a public read-only property returning a Func. I can see no good reason why this shouldn't be a method, although obviously the motivation for this depends on the implementation in the base class, which we cannot see in your question.

If you can modify the base class, I would recommend replacing this property with a virtual or abstract method. This property syntax instead is more obtuse for a maintenance programmer to read and is over-complex. Consider this instead:

public override LocationDataContract Resource(LocationContext context)
{
    var location = context.Locations.SingleOrDefault(l => l.Id == this.Identifier);
    return location == null ? 
        null : 
        new LocationDataContract
        {
            Id = location.Id,
            Coordinates = location.Coordinates,
            Description = location.Description,
            Name = location.Name,
            Type = location.Type,
            Details = this.GetDetails(location.Type, context)
        });
}

Additionally, the name Resource gives me no information. It might as well be called Foo. It looks like a function that grabs a location data contract from a location context based on the currently stored Id, so a name like "LocationContractForCurrentId" would make more sense.

Lastly, your usage of SingleOrDefault everywhere concerns me. If it's truly acceptable that nothing happens when no identifiers can be found, including in your context set methods, then fine, keep it that way, but if this is a way to hide from exceptions caused by invalid data or incorrect programming and to avoid Failing Fast, then that is a serious issue that would be better dealt with than hidden from.

StackExchange Code Review Q#84154, answer score: 2