{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/corefx/issues/14336","repository_url":"https://api.github.com/repos/dotnet/corefx","labels_url":"https://api.github.com/repos/dotnet/corefx/issues/14336/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/corefx/issues/14336/comments","events_url":"https://api.github.com/repos/dotnet/corefx/issues/14336/events","html_url":"https://github.com/dotnet/corefx/issues/14336","id":194465351,"number":14336,"title":"String should implement IReadOnlyList<char>","user":{"login":"danmosemsft","id":6385855,"avatar_url":"https://avatars3.githubusercontent.com/u/6385855?v=4","gravatar_id":"","url":"https://api.github.com/users/danmosemsft","html_url":"https://github.com/danmosemsft","followers_url":"https://api.github.com/users/danmosemsft/followers","following_url":"https://api.github.com/users/danmosemsft/following{/other_user}","gists_url":"https://api.github.com/users/danmosemsft/gists{/gist_id}","starred_url":"https://api.github.com/users/danmosemsft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/danmosemsft/subscriptions","organizations_url":"https://api.github.com/users/danmosemsft/orgs","repos_url":"https://api.github.com/users/danmosemsft/repos","events_url":"https://api.github.com/users/danmosemsft/events{/privacy}","received_events_url":"https://api.github.com/users/danmosemsft/received_events","type":"User","site_admin":false},"labels":[{"id":153714228,"url":"https://api.github.com/repos/dotnet/corefx/labels/api-needs-work","name":"api-needs-work","color":"207de5","default":false},{"id":174276033,"url":"https://api.github.com/repos/dotnet/corefx/labels/area-System.Runtime","name":"area-System.Runtime","color":"d4c5f9","default":false},{"id":145399958,"url":"https://api.github.com/repos/dotnet/corefx/labels/up-for-grabs","name":"up-for-grabs","color":"159818","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/dotnet/corefx/milestones/5","html_url":"https://github.com/dotnet/corefx/milestone/5","labels_url":"https://api.github.com/repos/dotnet/corefx/milestones/5/labels","id":1328080,"number":5,"title":"Future","description":"Milestone tracking issues not triaged to a planned release. Work in this milestone may happen, or may not: it is not committed.","creator":{"login":"joshfree","id":1302850,"avatar_url":"https://avatars1.githubusercontent.com/u/1302850?v=4","gravatar_id":"","url":"https://api.github.com/users/joshfree","html_url":"https://github.com/joshfree","followers_url":"https://api.github.com/users/joshfree/followers","following_url":"https://api.github.com/users/joshfree/following{/other_user}","gists_url":"https://api.github.com/users/joshfree/gists{/gist_id}","starred_url":"https://api.github.com/users/joshfree/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/joshfree/subscriptions","organizations_url":"https://api.github.com/users/joshfree/orgs","repos_url":"https://api.github.com/users/joshfree/repos","events_url":"https://api.github.com/users/joshfree/events{/privacy}","received_events_url":"https://api.github.com/users/joshfree/received_events","type":"User","site_admin":false},"open_issues":1228,"closed_issues":103,"state":"open","created_at":"2015-09-29T00:25:45Z","updated_at":"2017-07-28T15:25:10Z","due_on":null,"closed_at":null},"comments":17,"created_at":"2016-12-08T22:47:35Z","updated_at":"2017-07-28T20:18:33Z","closed_at":null,"body":"@stas-sultanov commented on [Mon Mar 23 2015](https://github.com/dotnet/coreclr/issues/542)\n\nI would like to suggest to make class String to implement IReadOnlyList{char} instead of IEnumerable{char}.\n\nPossibly interface should be implemented explicitly because of the Length property, which is extern and hardcoded in JIT. On the other hand Length could be deprecated and Count should be used instead.\n\n\n---\n\n@pharring commented on [Thu Mar 26 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-86749090)\n\n@stas-sultanov Thanks for the suggestion. Can you give an example of where this would be useful?\n\n\n---\n\n@stas-sultanov commented on [Fri Mar 27 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-86966435)\n\nSure. Here is an example from one of my projects:\nA ConvertEx class which provides a set of methods to convert data from base-n string to byte array and vice-versa. This class has a lot of methods like:\n\n``` C#\npublic static TryResult<String> TryToBase16String(byte[] data, int offset, int length)\npublic static TryResult<Byte[]> TryFromBase16String(string data, int offset, int length)\n```\n\nfor this methods i'd like to use single method that checks arguments\n\n``` C#\n    private static void CheckArguments<T>(IReadOnlyCollection<T> data, int offset, int length)\n    {\n        if (data == null)\n            throw new ArgumentNullException(@\"data\");\n        if (length < 0)\n            throw new ArgumentOutOfRangeException(@\"length\", length, @\"Is less than 0.\");\n        if (offset < 0)\n            throw new ArgumentOutOfRangeException(@\"offset\", offset, @\"Is less than 0.\");\n        if (offset > (data.Count - length))\n            throw new ArgumentOutOfRangeException(@\"data\", data.Count, @\"Offset plus length is greater than the length of data.\");\n    }\n```\n\nBut unfortunately it is not possible because byte[] and string has only one common interface `IEnumerable<T>`. Use `IEnumerable<T>` and get length via `Count()` method is not an option due to performance consideration.\n\n\n---\n\n@pharring commented on [Fri Mar 27 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-86985851)\n\nA non-generic overload of CheckArguments would solve this, of course.\n\n\n---\n\n@stas-sultanov commented on [Fri Mar 27 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-87003680)\n\nof course it would, and that it is the way it is made now. :) but it dose not looks like a good design.\n\nmaybe example is not good enough, however i strongly believe that this small change would made coreclr more versatile. \n\n\n---\n\n@SamuelEnglard commented on [Sun Mar 29 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-87472249)\n\nJust a quick point but unless I'm wrong array's don't implement `IReadOnlyList<T>` so you'd still have to use the `Enumerable.Count()`. Do access the length/count of both an array and string in an efficient manner we'd have to `Enumerable.Count()` smarter. It already looks for types that implement `ICollection` or `ICollection<T>` and uses the `Count` property instead of looping. If we add a shortcut for `IReadOnlyList<T>` to that too then you could do what you want.\n\n\n---\n\n@stas-sultanov commented on [Mon Mar 30 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-87485509)\n\nArrays do implement `IReadOnlyList<T>`, please take a look at comments of [Array](https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Array.cs) class.\nOr try to compile followin code:\n\n``` C#\nIReadOnlyList<int> test = new [] {1, 2, 3};\n```\n\nthe Enumerable class that implements Count method is not part of the coreclr.\n\n\n---\n\n@JohanLarsson commented on [Sun Mar 29 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-87487163)\n\n[Not much perf diff between .Count() and .Length for an array](http://referencesource.microsoft.com/#System.Core/System/Linq/Enumerable.cs,1191)\nIReadOnlyList<T> is a nice interface though.\n\n\n---\n\n@SamuelEnglard commented on [Sun Mar 29 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-87487299)\n\nAh yes, I had looked before posting but missed that comment. My bad. So then yes I'm all for making string implement `IReadOnlyList<T>`. Sorry about that\n\n\n---\n\n@stas-sultanov commented on [Mon Mar 30 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-87648799)\n\n> Not much perf diff between .Count() and .Length for an array\n\nindeed, but once again:\n\n> the Enumerable class that implements Count method is not part of the coreclr.\n\n\n---\n\n@VladimirGnenniy commented on [Mon Mar 30 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-87668687)\n\nI believe that this would be nice improvement of the architecture of the .net classes.\n\n\n---\n\n@terrajobst commented on [Tue Apr 07 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-90750642)\n\n@KrzysztofCwalina How do you think about this? It seems that `String` would eventually support `Span<char>`, would this additional interface make sense in general?\n\n\n---\n\n@Lukazoid commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113505393)\n\n+1 for this, would be really useful to be able to pass strings directly as IReadOnlyList<char>/IReadOnlyCollection<char> parameters instead of going via .ToCharArray() or using a wrapper class.\n\n\n---\n\n@KrzysztofCwalina commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113542312)\n\nTo me string is a primitive type. I think it's already a mistake that it implements IEnumerable<char>.\n\n\n---\n\n@sharwell commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113547344)\n\n@KrzysztofCwalina I totally get that view. It leads to [special cases](https://github.com/antlr/antlrcs/blob/bff09d7d1f25c64b680268d218e8461d4666501a/Antlr4.StringTemplate/Interpreter.cs#L1280-L1282) in certain code that isn't required in, [say, Java](https://github.com/antlr/stringtemplate4/blob/master/src/org/stringtemplate/v4/Interpreter.java#L1111-L1127). But for better or worse, that ship has sailed. Given that string already implements `IEnumerable<char>` (and that isn't going to change), the question is whether or not it should also implement `IReadOnlyList<char>`.\n\n:thought_balloon: I think it makes sense specifically for some cases where code currently needs to call `ToCharArray()`. I couldn't find any clear-cut cases where the new implementation would simplify my own existing code, but especially given it already implements `IEnumerable<char>` I think it makes sense to implement `IReadOnlyList<char>` as well.\n\n\n---\n\n@KrzysztofCwalina commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113549499)\n\nIf we implemented it explicitly and made sure the Linq extensions don't show on String, then I guess it would be fine. \n\n\n---\n\n@sharwell commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113570244)\n\n@KrzysztofCwalina It already implements `IEnumerable<char>`, so the LINQ extensions already appear.\n\n\n---\n\n@KrzysztofCwalina commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113573465)\n\nHmmm, I was under the impression that we special cased it in C#. \n\n\n---\n\n@KrzysztofCwalina commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113578131)\n\n@jaredpar, @MadsTorgersen, I thought C# was special cased to not show Linq methods on String?\n\n\n---\n\n@sharwell commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113578491)\n\nIn the PCL, `string` only implements `IEnumerable` (not `IEnumerable<char>`), so only `OfType` and `Cast` appear. When I target .NET 4.5+ desktop, I see the LINQ methods appearing in VS 2015 RC.\n\n\n---\n\n@MadsTorgersen commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113582793)\n\n@KrzysztofCwalina I think you just spent too much time with PCL. :-) In the full framework, LINQ always worked over string - for better or worse.\n\n\n---\n\n@KrzysztofCwalina commented on [Fri Jun 19 2015](https://github.com/dotnet/coreclr/issues/542#issuecomment-113583825)\n\nYeah, it's all slowly coming to me. I think we initially wanted to special case it, but then we just decided to implement IEnumerable. Then in 8.1 we added IEnumerable<char> (why?), and we are back to the original problem that string intellisense shows all these linq methods that probably should not be used on strings.\n\n\n---\n\n@danmosemsft commented on [Thu Dec 08 2016](https://github.com/dotnet/coreclr/issues/542#issuecomment-265878943)\n\nAPI request -- moving to CoreFX \n\n"},"comment":{"url":"https://api.github.com/repos/dotnet/corefx/issues/comments/318752146","html_url":"https://github.com/dotnet/corefx/issues/14336#issuecomment-318752146","issue_url":"https://api.github.com/repos/dotnet/corefx/issues/14336","id":318752146,"user":{"login":"jamesqo","id":9159214,"avatar_url":"https://avatars0.githubusercontent.com/u/9159214?v=4","gravatar_id":"","url":"https://api.github.com/users/jamesqo","html_url":"https://github.com/jamesqo","followers_url":"https://api.github.com/users/jamesqo/followers","following_url":"https://api.github.com/users/jamesqo/following{/other_user}","gists_url":"https://api.github.com/users/jamesqo/gists{/gist_id}","starred_url":"https://api.github.com/users/jamesqo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jamesqo/subscriptions","organizations_url":"https://api.github.com/users/jamesqo/orgs","repos_url":"https://api.github.com/users/jamesqo/repos","events_url":"https://api.github.com/users/jamesqo/events{/privacy}","received_events_url":"https://api.github.com/users/jamesqo/received_events","type":"User","site_admin":false},"created_at":"2017-07-28T20:18:33Z","updated_at":"2017-07-28T20:18:33Z","body":"What is the status of this proposal? @karelz, it looks like @joperezr has added a formal proposal [here](https://github.com/dotnet/corefx/issues/14336#issuecomment-270446899), maybe this is ready-for-review?"}}
{"actor":{"display_login":"jamesqo"}}