Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Conversation

Tratcher
Copy link
Member

@Eilon
Copy link
Contributor

Eilon commented Jan 27, 2015

Looks lovely :shipit:

public const int Status411LengthRequired = 411;
public const int Status412PreconditionFailed = 412;
public const int Status413RequestEntityTooLarge = 413;
public const int Status414RequestUurTooLong = 414;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uur?

@davidfowl
Copy link
Member

:shipit:

@kevinchalet
Copy link
Contributor

My 2 cents: the [Status] + code + name pattern is a bit hard to read and I'm not sure it's really useful: if you know the status code, you don't need to rely on a constant to use it. And if you wanna use a constant, that's often because you don't want to play with the status code directly.

@davidfowl
Copy link
Member

@PinpointTownes has a point, should me model it after the crazy System.Net enum?

@Tratcher
Copy link
Member Author

The enum naming assumed you (and everyone reading your code) were deeply familiar with the reason phrases and what the resulting status codes would be. If you just use reason phrases people have to guess the matching status code, where if you just use integers they have to guess the matching reason phrase.

I picked the Status404NotFound pattern because it lets you see both parts together for validation. It also lets you use intellisense like a lookup table where you can type in either the status code or the reason phrase and it immediately shows you the other half. It has Status on there because you can't start with digits.

Would you prefer NotFound404? I find that a little odd because it's the opposite of how it shows up in the response and spec. How about S404NotFound?

@Eilon
Copy link
Contributor

Eilon commented Jan 28, 2015

I like it just fine the way it is. I view the point of these as being to eliminate what appear to be magic numbers from the code. And I think this does that beautifully.

@Eilon
Copy link
Contributor

Eilon commented Jan 28, 2015

And for the record, I strongly dislike the System.Net StatusCode enum because it adds no value at all. It just takes one hard to understand piece of information and hides it behind another hard to understand piece of information.

@Tratcher Tratcher merged commit 096a0bf into aspnet:dev Jan 28, 2015
@Tratcher Tratcher deleted the statuscodes branch January 28, 2015 21:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants