nvaccess
Add constants for the TextInfo's compareEndPoints method
IN TextInfo.compareEndPoints it says: """ compares one end of this range to one end of another range. Subclasses must implement this. @param other: the text range to compare with. @type other: L{TextInfo} @param which: The ends to compare; one of "startToStart", "startToEnd", "endToStart", "endToEnd".
I propose creating constants of init.py to formalize these raw strings.
In general, I agree that constants are better than raw strings; errors can be caught more quickly. However, it also creates even more typing than there already is. If we're going to do this, I actually think we should go a bit further and do something I've been thinking about for a while.
I've been thinking about having .start and .end properties which are comparable, so you could just do: a.start == b.start, a.start > b.end, a.end <= b.start, etc. We could even go further and make assignment work as well: a.start = b.end. IMO, this is a lot easier to think about than method calls.
These properties wouldn't actually be "real" as such. They'd just have getters which create fake objects with comparison operators. They'd also have setters to handle assignment. This can all be done on the base TextInfo class and it would just call the original methods, thus making life easier for implementers.
One reservation here is that I really don't want to go through the entire code base replacing all occurrences of this. Even if someone else does it, there's a huge potential for error, especially if some sort of automation is used; there are always potential corner cases that can be missed. I think I'd rather just see this changed as code which uses it is updated.
Another reservation is that doing this adds one or two extra calls per comparison. I'm not sure if this would have a performance impact; I'm guessing it'd be negligible. However, it also adds two ways of doing the same thing, which could be confusing for callers.
CC @MichaelDCurran.
Is comparing strings more expensive than comparing integer constants in Python?
If the strings are references to the same object (which is the case for a constant), comparison is cheap, since Python can just compare object identity. In fact, even for literals, Python often "interns" strings, and when this occurs, they will be the same object. (That said, the rules for when strings are interned are somewhat convoluted.)
@MichaelDCurran and I discussed this. We agreed that it's better to do the constants and the .start/.end property thing separately. In other words, do the constant work here and we'll discuss the .start/.end thing at some future point.
So just assign each string to a constant?
On 5/17/2016 5:59 PM, James Teh wrote:
@MichaelDCurran https://github.com/MichaelDCurran and I discussed this. We agreed that it's better to do the constants and the .start/.end property thing separately. In other words, do the constant work here and we'll discuss the .start/.end thing at some future point.
— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/nvaccess/nvda/issues/5953#issuecomment-219888991
Derek Riemer
- Department of computer science, third year undergraduate student.
- Proud user of the NVDA screen reader.
- Open source enthusiast.
- Member of Bridge Cu
- Avid skiier.
Websites: Honors portfolio http://derekriemer.com Awesome little hand built weather app! http://django.derekriemer.com/weather/
email me at [email protected] mailto:[email protected] Phone: (303) 906-2194
@derekriemer, are you still considering to implement this? cc: @leonardder, @josephsl
It seems Derek is not active in the development anymore, and there is a common understanding on this request. cc: @seanbudd, @gerald-hartig I think updated triaging from NV Access side is needed here.
Personally, I think it's more appropriate to implement this as enums now that we're on a version of Python that supports them
It's worth noting that .compareEndPoints is not the privileged method anymore.
Indeed, instead of:
ti1.compareEndPoints(ti2, 'startToStart')
It's now recommended to use:
ti1.start == ti2.start
or any other comparison operator.
@CyrilleB79 it sounds like this issue is no longer relevant and can be closed.
While consumers have a new API (ti1.start == ti2.start), implementers still implement the old API (ti1.compareEndPoints(ti2, 'startToStart')). So, having constants/an enum might make this cleaner for implementers, but it seems like it's super low priority.