[API Proposal]: HttpMethod.From(string)

[stephentoub]

Background and motivation

HttpMethod provides a constructor (string method = ...; ... = new HttpMethod(method);), but the 99.999% case is that one of the static properties on HttpMethod suffices, and folks end up writing their own mapping tables to use those singletons. We can just provide that mapping once.

API Proposal

namespace System.Net.Http;

public class HttpMethod
{
// Case insensitive - to match our logic in Networking stack
+   public static HttpMethod From(ReadOnlySpan<char> method);
// Alternative names: Parse / Get / Create

//   public static HttpMethod From(string method); ... OPTIONAL, Networking team thinks it is NOT necessary

/*
        method switch
        {
            "GET" => s_getMethod,
            "PUT" => s_putMethod,
            "POST" => s_postMethod,
            "DELETE" => s_deleteMethod,
            "HEAD" => s_headMethod,
            "OPTIONS" => s_optionsMethod,
            "TRACE" => s_traceMethod,
            "PATCH" => s_patchMethod,
            "CONNECT" => s_connectMethod,
            _ => new HttpMethod(method);
        };
*/
} 

API Usage

string methodName = ...;
HttpMethod method = HttpMethod.From(methodName);

Alternative Designs

No response

Risks

No response

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

HttpMethod provides a constructor (string method = ...; ... = new HttpMethod(method);), but the 99.999% case is that one of the static properties on HttpMethod suffices, and folks end up writing their own mapping tables to use those singletons. We can just provide that mapping once.

API Proposal

namespace System.Net.Http;

public class HttpMethod
{
+   public static HttpMethod From(string method) =>
        method switch // could also decide to make this case-insensitive
        {
            "GET" => s_getMethod,
            "PUT" => s_putMethod,
            "POST" => s_postMethod,
            "DELETE" => s_deleteMethod,
            "HEAD" => s_headMethod,
            "OPTIONS" => s_optionsMethod,
            "TRACE" => s_traceMethod,
            "PATCH" => s_patchMethod,
            "CONNECT" => s_connectMethod,
            _ => new HttpMethod(method);
        };
} 

API Usage

string methodName = ...;
HttpMethod method = HttpMethod.From(methodName);

Alternative Designs

No response

Risks

No response

Author:	stephentoub
Assignees:	-
Labels:	api-suggestion, area-System.Net.Http, untriaged, needs-area-label
Milestone:	-

[ShreyasJejurkar]

I can take this one for implementation once it's approved! 🙌

[KeterSCP]

Have you thought about naming it Parse?

Yeah, I know that there is not much parsing involved, but still it seems to be more consistent with other APIs

[neon-sunset]

Since spans can now be matched against strings, From(ReadOnlySpan<char> method) overload might make sense too.
Alternatively, as @KeterSCP suggested, it might as well be better to implement ISpanParsable<HttpMethod> instead.

[karelz]

Triage: Makes sense, top post updated with discussion results.

[bartonjs]

Video

• We think that the method should be Parse instead of From.
• It's probably reasonable for the method to do case normalization and maybe trimming ("get " -> "GET")
• It's mildly unfortunate for the 0.01% case when it hits the unknown case that it could be string -> ROS-char -> new allocated string, but since it's expected to be 0.01%, it's fine.

namespace System.Net.Http;

public class HttpMethod
{
    public static HttpMethod Parse(ReadOnlySpan<char> method);
}

[MihaZupan]

and maybe trimming

If you give us garbage we could just let the constructor throw and have the caller fix their substring/slice 🤷‍♂️ Don't care too much about it though.

[Neme12]

Maybe it should implement IParsable/ISpanParsable as well if it has a Parse method? That just seems like general goodness so that serialization frameworks would automatically be able to deserialize this type.