[Monodevelop-devel] Text Editor API GetCharAt

[Mike Krüger]

Hi

I chose to stay with GetCharAt  because most people preferred it (only 
Jeff liked the indexer).

I don't think it's 'hard' to discover. The indexer makes sense IMO. It 
shouldn't be looked at the editor/document level - just at the 
ITextSource. That's very close to a char[] ... the document & editor is 
just extending that. I had to 'discover' that in the roslyn API ... took 
me about < 5 seconds. In the Roslyn version it's needed to use the 
Roslyn text API from time to time - then we would need to mix GetCharAt 
and Indexers anyways. This would be more consistent.

The GetCharAt OTOH is easier to read/in line with GetTextAt - and - I 
can't guarantee that it will stay O(1). One strong argument for staying 
with GetCharAt is that we need fewer code changes and we're all used to 
that.

btw. the API can still be changed. I would like to hear opinions - I 
just wanted to bring up one topic to start the discussion about that. 
Working with text is an important feature of the IDE.

Regards
Mike

> Thinking about the last message from Lluis, maybe conceptually the 
> better would be an indirection of sorts: a readonly property Chars 
> that implements an interface that is indexed? Performance-wise it can 
> be a problem if the jit can't optimize it into the equivalent 
> optimized form of GetCharAt, also null-checks, may be coded (not 
> necessarily needed, but coded) that may be hard to optimize out, too. 
> Under the wraps it would need to do all the buffer handling (maybe 
> with paging) that GetCharAt is indeed obliged to handle, maybe split 
> between the property getter and the indexer getter and setter.
>
> Just my 2 cents,
>
> Rafael Teixeira
> O..:.)oooo
>
>
> On Mon, Jun 23, 2014 at 6:44 AM, Lluis Sanchez Gual 
> <slluis.devel at gmail.com <mailto:slluis.devel at gmail.com>> wrote:
>
>>
>>     Hi
>>
>>     I'm currently reviewing the text editor API. We've a method called:
>>
>>     public char GetCharAt (int < span class="n" style="box-sizing:
>>     border-box; color: rgb(51, 51, 51);">offset)
>>
>>     And atm I think about replacing it with :
>>
>>     public  char  this[int  offset]  
>>
>>     Maybe even add a setter to that - then the editor may feel a bit
>>     more like a string from a certain PoV.
>>     The question is: Is that a good or bad choice - how are you
>>     feeling about it ?
>
>     Bad. It makes the api harder to discover (no code completion) and
>     harder to understand. Indexers should only be used for classes
>     which are collections (and a text editor is not a collection, it
>     just happens to contain a collection of chars, among many other
>     things).
>
>>
>>     The text editor has 3 layers: Text, Document and Editor - each
>>     layer adds something on top of the text. The GetCharAt is a
>>     functionality from the Text. On this level there is just text. I
>>     think the indexer makes sense there. But how about the upper
>>     layers? At Document level the editor becomes a collection of
>>     lines as well.
>>
>>     I would love to hear some opinions. (About the speed: In this is
>>     a very lightweight operation - in our current Implementation it's
>>     done in O(1) in the worst case I can imagine that it becomes an
>>     O(log N) one).
>>
>>     Regards
>>     Mike
>>
>>
>>     _______________________________________________
>>     Monodevelop-devel-list mailing list
>>     Monodevelop-devel-list at lists.ximian.com
>>     <mailto:Monodevelop-devel-list at lists.ximian.com>
>>     http://lists.ximian.com/mailman/listinfo/monodevelop-devel-list
>
>
>     _______________________________________________
>     Monodevelop-devel-list mailing list
>     Monodevelop-devel-list at lists.ximian.com
>     <mailto:Monodevelop-devel-list at lists.ximian.com>
>     http://lists.ximian.com/mailman/listinfo/monodevelop-devel-list
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ximian.com/pipermail/monodevelop-devel-list/attachments/20140623/86c8d2c8/attachment.html>