CODEWITHSUNDEEP
c#entity-frameworkrepository
Eric Packwood
Eric Packwood

Reputation: 1059

Better way of implementing a repository query

Here is what I have right now:

    public IEnumerable<Review.Project> GetProjectsByUser(int userID)
    {
        var user = _context.Users.Where(u => u.UserID == userID).FirstOrDefault();

        return user != null ? user.Projects : new List<Review.Project>();
    }

What I am wonder is if there is a better way to handle this.

  1. Should I check for null like I am?
  2. Should I throw an error instead of returning an empty collection?
  3. Is there a better way to get a list of projects that the user belongs to?

This is in my C# repository that uses Entity Framework.

Upvotes: 1

Views: 470

Answers (4)

scmccart
scmccart

Reputation: 1169

I would put a select before the .FirstOrDefault() to grab the Projects navigation property,

public IEnumerable<Review.Project> GetProjectsByUser(int userID)
{
    var projects = _context.Users
        .Where(u => u.UserID == userID)
        .Select(u => u.Projects)
        .FirstOrDefault();

    return projects != null ? projects : new Review.Project[0];
}

Upvotes: 0

KeithS
KeithS

Reputation: 71573

Well, you might be able to avoid the check for null by querying the Projects context directly, assuming you have a backreference to User:

public IEnumerable<Review.Project> GetProjectsByUser(int userID)
{
    return _context.Projects.Where(p => p.User.UserID == userID);        
}

That will return an empty enumerable if the userID is null, the user is not found in the database, OR if the user has no projects. It doesn't matter which, and none of them cause an error when running the query.

EDIT: If a Project has a collection of Users instead of just one, try this:

public IEnumerable<Review.Project> GetProjectsByUser(int userID)
{
    return _context.Projects.Where(p => p.Users.Any(u.UserID == userID));        
}

This will produce a slightly less performant, but still not bad, EXISTS query, joining to Users through the many-to-many cross-ref table.

Upvotes: 0

Naor
Naor

Reputation: 24083

In cases like this, I assume that the userID is exists and I would like the user to get an exception in case userID is invalid. In addition, userID should be unique so in case there is more then one userId - I would like to get an exception too in order to prevent wrong operations. So I would do:

public IEnumerable<Review.Project> GetProjectsByUser(int userID)
{
    var user = _context.Users.Where(u => u.UserID == userID).Single();
    //Now I am sure that user is not null

    return user.Projects;
}

But this is up to your notations.

So, my answers to you: 

Should I check for null like I am?

If there is a possibility that the user with userID is not exists - then yes. 

Should I throw an error instead of returning an empty collection?

In case there are no projects, you can return empty collection or null. I don't see any reason to throw an error. 

Is there a better way to get a list of projects that the user belongs to?

There are many ways that depends on you model but what you did is fine.

Upvotes: 1

SethO
SethO

Reputation: 2791

1) Yes, check for null. 2) No, unless a user w/o any projects is truly an error condition. Even then, I'd recommend logging an error and returning an empty collection. 3) I like what you're doing here. It's similar to other implementations I've seen. The only consideration you may to want to address is whether you want the user to receive an expression (as written) or an actualized collection (return user.Projects.ToList()).

Upvotes: 0

Related Questions