Mvc: Discussion for announcement "MVC's action result naming changes"

Created on 19 Feb 2016  路  16Comments  路  Source: aspnet/Mvc

Most helpful comment

The lack of consistency here is really frustrating. The status codes all have well defined names so naming seems like a non-problem. If other parts of the framework name things counter to the RFC and can't be changed for compatibility reasons that is fine but I think it is far more important for users to have some way to rely on consistent naming within a given context. When in the context of writing a Controller (one of the most common contexts in MVC I would imagine) and you need to return a status code the naming is all over the map.

For example, if I want to return an 200 OK with a payload I can do Ok(payload). If I want to return a 400 Bad Request with a payload I can do HttpBadRequest(payload). If I want to return a 409 Conflict with a payload it appears I have to manually construct a response and throw it as an exception.

There are relatively few status codes out there and they are all pretty well defined (at least as far as naming goes). It seems like it would be a pretty easy task to simply provide a set of result wrappers (like Ok or HttpBadRequest); maybe an hour or two of typing. Also, I would rather have both Ok and OkResult and HttpOk as options for backwards compatibility if it meant at least one of them had all of the status codes in that format. Boilerplate typing (like the Http prefix) doesn't bother me in the slightest, but having every one of my status code responses constructed in a different way is maddening. I have one controller action right now that has the following:

  • HttpBadRequest(payload)
  • new NoContentResult()
  • Manually constructed 409 Conflict with payload.

At this point I have a reasonably strong desire to just not use the framework supplied IActionResults and build my own set of helpers that are at least consistent.

All 16 comments

I am using RC1 and I personally miss few result types:

  1. 403 ForbiddenResult (I see it already exists in repository, named ForbidResult (why not Forbidden as in https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html ?))
  2. 405 MethodNotAllowedResult
  3. 409 ConflictResult

Is there any discussion why few of these response types are implemented, while others are not?

@zygimantas some context to the ForbidResult name choice - https://github.com/aspnet/Mvc/pull/3461#discussion_r43673290. It boils down to keeping it similar to the names AuthenticationManager uses - https://github.com/aspnet/HttpAbstractions/blob/dev/src/Microsoft.AspNetCore.Http.Abstractions/Authentication/AuthenticationManager.cs#L87

Just Google "http 403" . It should be forbidden IMO.

Also +1 for 405 and 409. Those make sense

@pranavkm thanks, it makes sense in general, but it conflicts with other response types, which are named accordingly to HTTP status codes. I would choose "Forbidden" name, no matter what name AuthenticationManageruses. In addition to that, "Forbid" is a verb.

Right now, I must use 3 different notations in a single project:

  1. new BadRequestResult() - because it is clearly readable.
  2. new HttpStatusCodeResult(409) - because GoneRequestResult does not exist.
  3. this.CreatedAtAction() - because of headers and payload.

Something like this might give more consistency:

c# StatusCodeResult( int statusCode, object content = null, collection_type? headers = null, int? subStatusCode = null)

And there might be an enumeration or class with constants which would translate readable status codes to int, i.e: StatusCodes.Forbidden to 403

Related issue: https://github.com/aspnet/HttpAbstractions/issues/174

Is there a particular reason that the Http prefix was dropped? Even though I'm familiar with status codes, I find it helpful to let intellisense help me pick from a list. I think this particular part of the change would also make things more opaque to a newcomer.

Thanks for making it more consistent. I also agree with @zygimantas that it should be "Forbidden". In the AuthenticationManager example "forbid" is used as verb (ForbidAsync = "forbid this right now in an async fashion"), not an adjective. It even "sounds" bad... "forbid (the) result" rather than "result of type http forbidden".

Is there a particular reason that the Http prefix was dropped?

This was never applied consistently in MVC or WebAPI. We started out trying to be consistent with the past releases (to avoid breaking your code), and embraced the fact that weren't ever consistent about it in previous versions. However, we've continued to get feedback about it to the point where it seemed worth the break.

Given a choice, we'd pick less boilerplate over more, so we dropped the prefix.

ForbidAsync = "forbid this right now in an async fashion"),

Good example.

Is there a particular reason that the Http prefix was dropped?

  • Given a choice, we'd pick less boilerplate over more, so we dropped the prefix.

If there is not real benefit, I would choose for one which is most used - that is with the prefix in previous ASP.NET versions.

Building on @zygimantas and @304NotModified, I would like to see a StatusCode convenience method.

@rynowak I understand the desire for less boilerplate, but having prefixes that cause all those classnames wasn't just boilerplate, it helped people find them.

I agree with @tuespetre - if the status code names are going to be harder to discover in intellisense/autocomplete (and they are now), then we should at least have an easier way to indicate a status code directly.

One thing isn't fully clear to me. Are those exceptions always bind to a http status code? If so, then prefixing the "http" isn't bad IMO as the namespace (Microsoft.AspNetCore.Mvc) isn't containing "HttpStatuses" or something like that.

(edit, maybe I have to rename my username ;))

@aggieben

I agree with @tuespetre - if the status code names are going to be harder to discover in intellisense/autocomplete (and they are now), then we should at least have an easier way to indicate a status code directly.

In my opinion Visual Studio should sort all the possible options based on what's the most appropriate / likely item. For example, if you're in a function that returns IActionResult then it should first list all the available methods or classes that actually implement IActionResult. Kind of silly to show everything in alphabetical order...

The lack of consistency here is really frustrating. The status codes all have well defined names so naming seems like a non-problem. If other parts of the framework name things counter to the RFC and can't be changed for compatibility reasons that is fine but I think it is far more important for users to have some way to rely on consistent naming within a given context. When in the context of writing a Controller (one of the most common contexts in MVC I would imagine) and you need to return a status code the naming is all over the map.

For example, if I want to return an 200 OK with a payload I can do Ok(payload). If I want to return a 400 Bad Request with a payload I can do HttpBadRequest(payload). If I want to return a 409 Conflict with a payload it appears I have to manually construct a response and throw it as an exception.

There are relatively few status codes out there and they are all pretty well defined (at least as far as naming goes). It seems like it would be a pretty easy task to simply provide a set of result wrappers (like Ok or HttpBadRequest); maybe an hour or two of typing. Also, I would rather have both Ok and OkResult and HttpOk as options for backwards compatibility if it meant at least one of them had all of the status codes in that format. Boilerplate typing (like the Http prefix) doesn't bother me in the slightest, but having every one of my status code responses constructed in a different way is maddening. I have one controller action right now that has the following:

  • HttpBadRequest(payload)
  • new NoContentResult()
  • Manually constructed 409 Conflict with payload.

At this point I have a reasonably strong desire to just not use the framework supplied IActionResults and build my own set of helpers that are at least consistent.

Yes, please make it consistent.

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.

Was this page helpful?
0 / 5 - 0 ratings