Skip to content

Span extension method to slice off of a string instance #23569

Closed

Description

@jnm2

Current API proposal

TODO (see https://github.com/dotnet/corefx/issues/24072#issuecomment-349412729): Add array overloads. Consider ArraySegment overloads for consistency.

public static class MemoryExtensions
{
    public static ReadOnlySpan<char> AsReadOnlySpan(this string text, int start);
    public static ReadOnlySpan<char> AsReadOnlySpan(this string text, int start, int length);
    public static ReadOnlyMemory<char> AsReadOnlyMemory(this string text, int start);
    public static ReadOnlyMemory<char> AsReadOnlyMemory(this string text, int start, int length);
}

Original post

I expected to see these:

class System.SpanExtensions
{
    public static ReadOnlySpan<char> Slice(this string text, int start);
    public static ReadOnlySpan<char> Slice(this string text, int start, int length);
}

But now I also see a proposal for a non-stack-only StringSegment type. So I have two questions.

  1. Is there a clearly preferable return type for such an extension method?
  2. Is the most efficient way to slice a string str.AsReadOnlySpan().Slice(...), or is there a single method call I can make that I haven't found yet? That's a little verbose for my liking, even if the two calls are inlined.

Activity

  1. karelz commented on Sep 15, 2017

    @karelz
    Member
  2. stephentoub commented on Sep 15, 2017

    @stephentoub
    Member

    cc: @KrzysztofCwalina, @ahsonkhan

    Is the most efficient way to slice a string

    Currently, yes.

    I expected to see these

    We've discussed adding such overloads. @KrzysztofCwalina would likely know the current plan.

    Is there a clearly preferable return type for such an extension method?

    For now StringSegment doesn't exist. I expect we'll have such discussions as part of considering adding it.

  3. ahsonkhan commented on Sep 15, 2017

    @ahsonkhan
    Contributor
    1. Is the most efficient way to slice a string str.AsReadOnlySpan().Slice(...), or is there a single method call I can make that I haven't found yet? That's a little verbose for my liking, even if the two calls are inlined.

    Yes, that is how you would slice a string. I would imagine you would grab a span from a string once (i.e. call str.AsReadOnlySpan()) and then pass that span around and slice multiple times using span.Slice().

    cc @KrzysztofCwalina

  4. KrzysztofCwalina commented on Sep 15, 2017

    @KrzysztofCwalina
    Member

    Last time I discussed this with @jaredpar and @MadsTorgersen the conclusion was that we need to be super sure that ReadOnlySpan<char> is the best representation for string slices to take the plunge and bur the best name (Slice) for a method returning ROS<char>.

    I think there are two cadidate types for the repserantation: ROS<char> and something like ReadOnlyMemory<char>. The former is safe and fast, but with all the restrictions of ref-like types. The later is subject to tearing, slower, but convinient to use.

  5. KrzysztofCwalina commented on Nov 28, 2017

    @KrzysztofCwalina
    Member

    We have gotten lots of feedback that the current str.AsReadOnlySpan().Slice(from, length) is to verbose.
    Maybe a good compromise would be to add:

    public static class MemoryExtensions
    {
        public static ReadOnlySpan<char> AsReadOnlySpan(this string text, int start);
        public static ReadOnlySpan<char> AsReadOnlySpan(this string text, int start, int length);
        public static ReadOnlyMemory<char> AsReadOnlyMemory(this string text, int start);
        public static ReadOnlyMemory<char> AsReadOnlyMemory(this string text, int start, int length);
    }

    @MadsTorgersen, @jaredpar, @stephentoub, @ahsonkhan?

  6. stephentoub commented on Nov 28, 2017

    @stephentoub
    Member

    I'd be fine with such overloads.

  7. KrzysztofCwalina commented on Nov 28, 2017

    @KrzysztofCwalina
    Member

    @jnm2, could you please update the API design at the top of this issue? ... I will mark it as ready for API review then.

  8. jnm2 commented on Nov 28, 2017

    @jnm2
    ContributorAuthor

    Oh, forgot this was one I started. 😄 Done.

  9. terrajobst commented on Dec 5, 2017

    @terrajobst
    Contributor

    Video

    That seems like a good suggestion, but we should do the same for arrays.

  10. karelz commented on Dec 5, 2017

    @karelz
    Member

    There seem to be also similar AsReadOnlySpan() extension for ArraySegment.
    @KrzysztofCwalina does it make sense to make it consistent with string and array overloads we are adding here?

  11. karelz commented on Dec 5, 2017

    @karelz
    Member

    FYI: The API review discussion was recorded - see https://youtu.be/BI3iXFT8H7E?t=3913 (13 min duration)

  12. ghost
    self-assigned this
    on Jan 4, 2018
  13. 2 remaining items

  14. ghost closed this as completedon Jan 19, 2018
  15. transferred this issue fromdotnet/corefxon Jan 31, 2020
  16. added this to the 2.1.0 milestone on Jan 31, 2020
  17. ghost locked as resolved and limited conversation to collaborators on Dec 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions