Convert 'StatusCodes' class to 'StatusCode' enumeration

[hishamco]

@Tratcher I notice from your PR #182 that StatusCodes are class the contains a constants, i think it will be nice to convert it to enumeration while all the values represents HttpStatusCode

[Tratcher]

No, we decided not to use an existing enum (System.Net.HttpStatusCode). HTTP status codes are a poor fit for enums. It's an open set of information, not a finite set. There are also some duplicate values possible.

[hishamco]

How it possible to have some duplicate values, while it represent Http status codes?!!

[davidfowl]

@hishamco why does it need to be an enum?

[Tratcher]

See https://msdn.microsoft.com/en-us/library/system.net.httpstatuscode(v=vs.110).aspx and look for 301 and 302 which each have two definitions.

[hishamco]

@davidfowl while all the constants in StatusCodes represent an Http status code, so enumeration is suitable type in CLR to represent such values. I know it's possible to implement such behavior using constants, but that is why enum is used for.
In my PR here https://github.com/aspnet/Hosting/pull/275/files is a suitable case where constants are preferable rather than enum, because the values are open set

[hishamco]

@Tratcher according to the RFC here http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html, each code represents only one value, I'm not sure why there are in System.Net.HttpStatusCode?!!

[hishamco]

If you have a look in our new APIs here https://github.com/aspnet/HttpAbstractions/blob/dev/src/Microsoft.AspNet.Http.Abstractions/StatusCodes.cs#L17 301 Moved no longer exist while 301 MovedPermanently there

[Tratcher]

You're ignoring the part about open vs closed sets. Http status codes can be anything from 100 to 599 (?). While the rfc species a few it allows for more to be added. It also allows for non-standard codes to be used. Enums work for complete sets of data, but they do not work well for incomplete sets like this where users would have to cast to fill in the gaps.

[hishamco]

I didn't ignore the open vs close sets 😄 I'm with you @Tratcher that enum is suitable for closed set, nothing but I saw several implementation such as .NET, Java .. etc that used Http status codes as enumeration ..

[Eilon]

They're still available as an enum from System.Net - you just have to cast to int to use them. And that's why we created a new one. Also, I think everyone here agrees the names of the new type are better than the existing enum because they include both the status code number and the short name.

[hishamco]

@Eilon i'm still thinking why you didn't use the existing enum while you can get the status names and codes too, but if we agreed about the current type I have another two suggestion, first of all singular name may better than plural StatusCode have a look to my PR here https://github.com/aspnet/Hosting/pull/275/files, second thing it will be nice if we modify the class to something like

public static class StatusCode
{
        public const int OK = 200;
        public const int Created = 201;
        public const int Accepted = 202;
        public const int NonAuthoritative = 203;
        public const int NoContent = 204;
        public const int ResetContent = 205;
        public const int PartialContent = 206;
        ...
}

because the dev he/she knows that he using StatusCode

[Eilon]

We want a type that has values that describe both the status code description and its numeric value. There is no existing type that does this.

[hishamco]

I see .. thanks for clarification @Eilon

[khellang]

I still think the smurf naming is a bit too much though... StatusCodes.Status200OK? It wouldn't hurt to leave out some StatusStatus? 😉

[Eilon]

@khellang do you have a suggestion for that? Identifiers can't start with integers unfortunately. And we don't like StatusCodes.OK200 either because we feel having the number come first is more usable.

[khellang]

Ouch. Forgot about that... Well, then I guess you're out of options 😞

BTW, have you thought about a using an implicitly convertible struct, simply wrapping the int value, for discoverability of the constants? E.g. "Do I find the constants in HttpStatusCode, StatusCode, StatusCodes, HttpStatus, or just use an int? I can't remember. Off to find the docs.".

[Eilon]

Not sure what you mean by that - can you show an example?

[khellang]

Consider this:

[DebuggerDisplay("{Value, nq}")]
public partial struct HttpStatusCode : IEquatable<HttpStatusCode>
{
    public HttpStatusCode(int value)
    {
        Value = value;
    }

    public int Value { get; }

    public bool Equals(HttpStatusCode other) => Value.Equals(other.Value);

    public override bool Equals(object obj) => !ReferenceEquals(null, obj) && (obj is HttpStatusCode && Equals((HttpStatusCode) obj));

    public override int GetHashCode() => Value;

    public override string ToString() => Value.ToString();

    public static bool operator ==(HttpStatusCode left, HttpStatusCode right) => left.Equals(right);

    public static bool operator !=(HttpStatusCode left, HttpStatusCode right) => !left.Equals(right);

    public static implicit operator HttpStatusCode(int value) => new HttpStatusCode(value);

    public static explicit operator int(HttpStatusCode statusCode) => statusCode.Value;
}

public partial struct HttpStatusCode
{
    public static readonly HttpStatusCode Status200OK = 200;
    public static readonly HttpStatusCode Status201Created = 201;
    public static readonly HttpStatusCode Status202Accepted = 202;
    public static readonly HttpStatusCode Status203NonAuthoritative = 203;
    public static readonly HttpStatusCode Status204NoContent = 204;
    public static readonly HttpStatusCode Status205ResetContent = 205;
    public static readonly HttpStatusCode Status206PartialContent = 206;

    // -- Snipped for brevity
}

If the public APIs exposed HttpStatusCode instead of int, it would provide a hint, as well as IntelliSense, to the developer that you can find the different status codes on the HttpStatusCode type.
This would be a nice substitute for an enum, without breaking the "open vs closed sets" requirement.

Another benefit with this, is that you can either add additional members to it yourself, or let people hang extension methods off it:

public static bool IsSuccess(this HttpStatusCode statusCode)
{
    return statusCode.Value >= 200 && statusCode.Value < 300;
}

This would be even nicer if/when C# supports extension properties 😄

Have you done a poll or something on what people would like the status properties to look like?
I actually think I'd rather have just OK (with XML docs saying it's 200) than Status200OK. If I already knew the numeric value, and assumed everyone else reading the code knows the HTTP status, I'd just use 200.

If you prefer name and numeric value, there's nothing stopping you from having your own "enum":

public static class MyAwesomeStatusCodes
{
    public static readonly HttpStatusCode Status420EnhanceYourCalm = 420;
}

[henkmollema]

I like that. 😄 Although the amount of code for such a simple type is a bit much, but that might not be a big issue.

[khellang]

BTW, here's a quick Twitter poll 😉 https://twitter.com/khellang/status/632105286870896640

Although the amount of code for such a simple type is a bit much, but that might not be a big issue.

Yeah, blame C# for that. http://fsharpforfunandprofit.com/posts/fsharp-decompiled/ 😜
This is boilerplate framework code, so it doesn't matter much.

[AnthonySteele]

Not really clear why you would "want a type that has values that describe both the status code description and its numeric value".

Some prefer the numbers, some prefer the name, but can you please everyone with a compromise? Including both is sufficiently ugly and verbose to put people from both those camps off. I really don't sign up to the notion that "everyone here agrees the names of the new type are better than the existing enum because they include both the status code number and the short name" and repeats the word "Status" for good measure.

There is a learning curve here, and either those who prefer the number will have to climb it, or other who prefer then name will have to deal with it. Personally I favour using the name only in the constant/enum, with option to cast from int if that's your preference,

[markrendle]

In Simple.Web (which is discontinued because MVC 6 is so much like it) I used a struct with implicit casting as in @khellang's example: https://github.com/markrendle/Simple.Web/blob/master/src/Simple.Web/Status.cs

It includes methods for any status which includes a Location header, e.g. Status.Created(resourceUrl).

I don't like the Status200OK format for names, not least because it's not IntelliSense-friendly. If people want to use numbers, they can just use an int.

[darrencauthon]

So the use of an int to represent an int is a "primitive obsession?"

I've seen this term "primitive obsession" pop up more and more... always used as as pejorative, casting the use of a simple type to represent a simple value as too simplistic.

Here's a story:

Bob: Frank, why are you using string a string for the user's name?
Frank: Because that's what the user entered, they just typed their name in the box and...
Bob: No!!! What is your obsession with using data this way, you're always like this! A better solution is this Name class I built, it'll give you so much more, look here...

And here's the topper:

Frank: Although the amount of code for such a simple type is a bit much, but that might not be a big issue.
Bob: Yeah, blame C# for that. Maybe we should consider switching to F# while we're at it?

Yeah, blame the language for forcing us to write all this boilerplate code... to wrap the simplest of values in a big abstraction of my own design!

If there's an obsession here, it's one in which programmers feel they must model everything with code. Just stop and look, as we're going from

statusCode == 200

to

statusCode == Microsoft.AspNet.Http.StatusCodes.Status200OK

to now

public static bool IsOk(this MySuperiorStatusCode statusCode)
{
    return statusCode.Value == HttpStatusCode.Status200Ok;
}

var mySuperiorStatusCode = new MySuperiorStatusCode(statusCode);
mySuperiorStatusCode.IsOk()

Look how simple that is to represent the "ok?"

For the record, I like the thinking of the @hishamco's call to swap Status200OK to OK. Look, we're adults writing code dealing with status codes. Leave it to our unit tests to deal with status code 200 as an input to our application, and let's make our production code as simple to read and write as possible. OK beats Status200OK for that.

[khellang]

@darrencauthon For the record; nothing in your example is true, even with the type suggested, you can still do statusCode == 200. All the other stuff is just hyperbolic bullshit 😄

It's totally up to you which, if any, extension methods you want to hang off it. My point is that now you can, beacuse you have a specific type for it.

So the use of an int to represent an int is a "primitive obsession?"

You're not representing a random int, you're representing an HTTP status code. There are different semantics.

let's make our production code as simple to read and write as possible.

This is exactly my point, the proposed type

1. Gives you nice IntelliSense for IANA registered HTTP status codes (easier to write)
2. Gives those who want to use the "enum" way a nice API, i.e. HttpStatusCode.Created (easier to read, for some)
3. Allows those who want to use int to do exactly that (easier to read, for others)
4. Enables you to add custom extension methods to the type, i.e. if (statusCode.IsServerError()) { /* do something */ } (easier to read)

✨👯✨

[Eilon]

We still feel the current design and API names are a good balance of usability and flexibility.

In my view a problem with introducing types (e.g. the struct) is that it ends up not being clear that you can use just an int because implicit operators are not very obvious.

There's no "silver bullet" here, obviously, but in our view the current approach has the greatest value and the fewest downsides.

[khellang]

You really feel that outweighs the benefits of a separate type? You don't even need to use an int, you can use the static values. And once you need/want to use an int, you learn that you can use it by reading docs or a simple google search, once, and have nice IntelliSense forever. I'm not sure "implicit operators are not very obvious" is what you should worry about. Anyway, it was worth the shot. At least we got a good Twitter discussion on it and a general consensus that including both name and numeric value sucks. 😄

[Eilon]

The current design makes the consts entirely optional. People are free to use the consts, existing enums (just have to cast to int), or a new type such as the one you propose.

[Tratcher]

Backreference: #174