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<CustomBehaviour> Get(int? orgID = null, long? userID = null)
{
using (PAMDatabase db = new PAMDatabase(WebApiConfig.PAMConnectionString))
{
List<CustomBehaviour> output = new List<CustomBehaviour>();
// 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<List<CustomBehaviour>>(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<List<CustomBehaviour>>(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.
.ToList()must not return nulls; it should return a list, which can be empty or with items. \$\endgroup\$cbsis aList<customBehaviour>, which is an entity object.CustomBehaviouris the corresponding Data Transfer Object. \$\endgroup\$if ((cbs != null) && (cbs.Count > 0))all together..ToList()will never return null, and adding an empty list tooutputis something you don't need to optimize. Compared to the time it takes to query the database, it's nothing. \$\endgroup\$