LINQ filtering query

I often find myself writing LINQ queries like this. I end up testing for Null and zero-counts pretty often, and the code sprawls out to be something like the following:

// GET: api/CustomBehaviours
public IEnumerable Get(int? orgID = null, long? userID = null)
{
    using (PAMDatabase db = new PAMDatabase(WebApiConfig.PAMConnectionString))
    {
        List output = new List();

        // Add all custom behaviours for the specified user
        if (userID != null) {
            var usr = db.context.users.AsNoTracking().Where(u => u.userID == userID).FirstOrDefault();
            if (usr != null)
            {
                var cbs = usr.customBehaviours.ToList();
                if ((cbs != null) && (cbs.Count > 0))
                    output.AddRange(Mapper.Map>(cbs));
            }
        }

        // Add all custom behaviours for the specified organisation
        if (orgID != null)
        {
            var org = db.context.orgs.AsNoTracking().Where(o => o.orgID == orgID).FirstOrDefault();
            if (org != null)
            {
                var cbs = org.customBehaviours.ToList();
                if ((cbs != null) && (cbs.Count > 0))
                    output.AddRange(Mapper.Map>(cbs));
            }
        }

        return output;
    }
}

Can this be made more concise? I'm tempted to remove all the Null checks and wrap the whole thing in a big try { } catch {} but I believe that goes against the conventional use of exceptions.

FirstOfDefault can also take a filtering query, just so you know.

Which means :

db.context.users.AsNoTracking().Where(u => u.userID == userID).FirstOrDefault();

can be :

db.context.users.AsNoTracking().FirstOrDefault(u => u.userID == userID);

If you use C#6, you can use the null propagation operator ?. instead of multiple null checks :

if (usr != null)
{
    var cbs = usr.customBehaviours.ToList();
    if ((cbs != null) && (cbs.Count > 0))
        //...

is now :

if (usr?.customBehaviours != null && cbs.Count > 0)

As pointed @ANeves in the comments, ToList never returns null. It'll return an empty collection or will throw an ArgumentNullException if the enumerable is null.

So, doing .ToList() before checking if usr.customBehavious == null won't save you.

I feel like you should split both your if in different methods, maybe GetUserCustomBehavior and GetOrganizationCustomBehavior. Then, your code woudl be cleaner.

// GET: api/CustomBehaviours
public IEnumerable Get(int? orgID = null, long? userID = null)
{
    using (PAMDatabase db = new PAMDatabase(WebApiConfig.PAMConnectionString))
    {
        List output = new List();

        // Add all custom behaviours for the specified user
        if (userID != null) {
            var userBehaviors = GetUserCustomBehaviors(userID.Value, db);
            output.AddRange(Mapper.Map>(userBehaviors));
        }

        // Add all custom behaviours for the specified organisation
        if (orgID != null)
        {
            var orgBehaviors = GetOrganizationCustomBehavior(orgID.Value, db);
            output.AddRange(Mapper.Map>(orgBehaviors));
        }

        return output;
    }
}

private IEnumerable GetUserCustomBehavior(int? userId, PAMDatabase db)
{
    var usr = db.context.users.AsNoTracking().FirstOrDefault(u => u.userID == userID);

    return usr?.customBehaviours ?? new List();
}

private IEnumerable getOrganizationCustomBehavior(int orgId, PAMDatabase db)
{
    var org = db.context.orgs.AsNoTracking().FirstOrDefault(o => o.orgID == orgID);

    return org?.customBehavior ?? new List();
}

Now, notice something, I wasn't able to see which type was used in your code because of the var keyword. var is cool, but tools like Resharper indicate they should be used only when you can see which type is used ex :

//good
var list = new List();
//not good
var user = FooTheBar();

Also, you don't really need to check for Count > 0, what's the worst that can happen, you'll add an empty range to your list? That's usually not important, and your code has less if, which is good.

I'm no pro with Automapper, but I don't think it's a good practice to map lists, maybe you should define your map per object (Mapper.CreateMap()) but maybe there's a performance gain I'm not aware of.

StackExchange Code Review Q#106148, answer score: 2