Docs: What is the usage guideline for span parsing methods consuming variable amount of bytes?

Created on 4 Mar 2020  Â·  12Comments  Â·  Source: dotnet/docs

Here is my usecase, I am creating a class called PubKey which represent a public key in my system.
I need to deserialize the bytes into a PubKey the bytes can be either 33 bytes or 65 bytes. The consumer need to know how many bytes has been consumed from the span.

What is the best way?

Producer:

public static bool TryParse(ReadOnlySpan<byte> input, out int length)
{
}

Consumer:

if (PubKey.TryParse(span, out length))
{
  span = span.Slice(length);
  ...
}

Or
Producer:

public static bool TryParse(ref ReadOnlySpan<byte> input)
{
   input = input.Slice(33);
}

Consumer

if (PubKey.TryParse(ref span))
{
  ...
}

The second method seems to be less intuitive.


Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

guide - Framework Design Guidelines Area - .NET Guide P2 doc-enhancement

Most helpful comment

The correct form would be

C# public static bool TryDecode(ReadOnlySpan<byte> source, out PubKey value, out int bytesConsumed) { ... }

  • Decode instead of Parse because it's interpreting bytes instead of text.
  • out int bytesConsumed is the answer for your original question.

    • If it isn't bytes, charsConsumed or valuesConsumed.

The bytesConsumed/charsConsumed/valuesConsumed portion (having the out parameter and the preferred names for the parameter) is explicitly part of the OperationStatus API pattern; then it generalizes backwards to Try-Read methods. These will be included as part of the new guidelines update in summer 2020.

All 12 comments

ping @tdykstra

@GrabYourPitchforks @adamsitnik Can you help with this question?

cc @bartonjs

Personally I like the Utf8Parser API which decided to use the first approach:

bool TryParse(ReadOnlySpan<byte> source, out byte value, out int bytesConsumed);

IMHO it's more intuitive: every C# dev should be already familiar with out (parsing data etc), I am not so sure about proper usage of refs, which can be tricky and also limiting (the input reference must be mutable)

The correct form would be

C# public static bool TryDecode(ReadOnlySpan<byte> source, out PubKey value, out int bytesConsumed) { ... }

  • Decode instead of Parse because it's interpreting bytes instead of text.
  • out int bytesConsumed is the answer for your original question.

    • If it isn't bytes, charsConsumed or valuesConsumed.

The bytesConsumed/charsConsumed/valuesConsumed portion (having the out parameter and the preferred names for the parameter) is explicitly part of the OperationStatus API pattern; then it generalizes backwards to Try-Read methods. These will be included as part of the new guidelines update in summer 2020.

In addition to what Jeremy said, we've been trying to shy away from APIs that auto-slice an input span and return the sliced span to you. (In your original proposal, those would be the _ref_ APIs.)

The reason for this is that the original call site may not have had a Span<T>. They instead may have had a T[], a string (for _T = char_), an ArraySegment<T>, a Memory<T>, or some other data type. When an API returns a sliced span it's not always intuitive as to how to map that return value back to the original data structure so that the caller can then slice the larger structure appropriately. So our guidance for these scenarios is to _out_ an integer or to use Range. This allows the caller to slice their own data structures correctly.

thanks a lot, I will follow the Utf8Parser then. It seems to me more intuitive as well.

I think this should not be closed until the docs is documenting it.

The guidance is part of Framework Design Guidelines; which is managed by a different process than the docs site in general. There's nothing that the people who work in this repository can really do about it.

@bartonjs Is there a repository or place to file issues?

Not yet. There's a vague plan, but nothing that can really be acted on until the summer (for lawyerly reasons).

@bartonjs I don't think it would hurt to also put it in the doc page in https://docs.microsoft.com/en-us/dotnet/standard/memory-and-spans/memory-t-usage-guidelines?view=netcore-3.1

That's what I googled when I was searching guidance.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Eilon picture Eilon  Â·  3Comments

skylerberg picture skylerberg  Â·  3Comments

ike86 picture ike86  Â·  3Comments

LJ9999 picture LJ9999  Â·  3Comments

sebagomez picture sebagomez  Â·  3Comments