Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add style for returning empty collections instead of null #30

Open
matiasbeckerle opened this issue Jun 2, 2017 · 6 comments
Open

Add style for returning empty collections instead of null #30

matiasbeckerle opened this issue Jun 2, 2017 · 6 comments
Assignees

Comments

@matiasbeckerle
Copy link
Contributor

https://blogs.msdn.microsoft.com/ericlippert/2009/05/14/null-is-not-empty/

@AlphaGit
Copy link
Member

AlphaGit commented Jun 2, 2017

I think this requires a detailed guideline, if we decide to add one.

Think of this:

GET /myCollection/1

  • If the collection 1 does not exist, the result should be null (or even better, 404)
  • If the collection 1 is empty, the result should be an empty collection (not null)

On another example (this one is not a collection, but contains one):

GET /itemWithCollections/1

  • If the item does not exist, the result should be null (or even better, 404)

  • If the item exists and does NOT have a collection, the collection should be null (not empty)

    {
        "ItemName": "Item Number 1",
        "Id": 1,
        "TheCollection": null
    }
  • If the item exists and the collection is does not have elements, the collection should be empty (not null)

    {
        "ItemName": "Item Number 1",
        "Id": 1,
        "TheCollection": []
    }

I think this is in agreement with Eric Lippert's post.

Also, this also applies to databases and methods, I just gave a web example.

@mvazquez-ms
Copy link
Contributor

mvazquez-ms commented Jun 2, 2017

Think of this:

I would prefer to avoid such example and limit the guideline to C# methods returning collections.

This proposal was triggered by a PR where a collection of users was being filtered by group. On that domain, it doesn't matter whether the group exist or not. Aligned with Lippert's post, you can assert that there are no users (Zero, empty collection) for a non existent group. Different would be if for some reason the query fails or something unexpected happen, then it would be OK to return null.

A 404 would be OK if you are asking for the details of that group. For a non existent group, you have no idea what the details are, but you do know for sure you have no users for that group.

My point is, as a rule for methods returning a collection, it's almost a standard on C#. For the scenario you are describing, I wouldn't enforce one way or the other.

@matiasbeckerle
Copy link
Contributor Author

Mmm I agree with @AlphaGit's comment. What you are saying @mvazquez-ms sounds like different of what we interpreted of the blog post. Can you clarify a little more?

@mvazquez-ms
Copy link
Contributor

Mmm I agree with @AlphaGit's comment. What you are saying @mvazquez-ms sounds like different of what we interpreted of the blog post. Can you clarify a little more?

Sure, what I mean is that the difference between 404 and 200 (with empty list) is whether you can (or not) assure there are 0 items. In certain domains, you can assure it regardless the existence of the parent resource.

The scenario that triggered this discussion was this:

`Get me the list of users for a certain group'

The implementation provided on the PR was like GET api/groups/{groupId}/users, and the first reaction is 'Ok, this should throw a 404 if groupId is not defined on DB', and yes, that make sense.

However, that PR was discarded because the project required something like GET api/users?groupId={groupId}, and in that case, you wouldn't throw a 404 if the groupId does not exist.

So, the requirement is the same, 'get users from a certain group'. The domain dictates the existence or not of the group is irrelevant, as it will be use as an optional filter. I would believe we all agree up to this point.

Now, the decision between implementing such requirement as:

GET api/groups/{groupId}/users
vs
GET api/users?groupId={groupId}

Is a matter of convention, preference, taste or whatever. We (@AlphaGit, @matiasbeckerle and I ) worked recently on a project that didn't follow REST standards, we can discuss whether it was right or wrong, but I do prefer to keep that discussion within the scope of a project instead of enforcing a guideline.

Now, if you define an endpoint like GET api/groups/{groupId}/users, well, yes, it's expected to require the existence of the group, but that's a whole different debate, and it's about REST standards, not C#.

TL;DR;

I would prefer to avoid such example and limit the guideline to C# methods returning collections.

@matiasbeckerle
Copy link
Contributor Author

Understand. I think the difference is in:

...as it will be use as an optional filter.

So if we are talking of methods that receives optional filters and returns a collection, then yes, I agree and such method should return an empty collection.

@AlphaGit
Copy link
Member

AlphaGit commented Jun 3, 2017

I see the difference, and I agree. In that case, group is not an entity in itself, but rather a filter that you're applying on a list of users.

When group is an entity: if you try to get the elements of the group, and group does not exist, you get a 404 or a null response.

When group is an attribute and you're just filtering another collection (users): you get an empty list. (Rationale: there are no users with that particular condition.)

I think it's yet another distinction that I hadn't considered before -- but I agree with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants