[GitHub] [accumulo] DomGarguilo commented on pull request #3342: Add RowRange object

["DomGarguilo (via GitHub)" <gi...@apache.org>]

DomGarguilo commented on PR #3342:
URL: https://github.com/apache/accumulo/pull/3342#issuecomment-1538645691

   > Looking at this it would be really nice if [Range](https://github.com/apache/accumulo/blob/56d49f15a05db9a46dbceb845918497760601c11/core/src/main/java/org/apache/accumulo/core/data/Range.java#L39) and `RowRange` could share code. Two ideas I had:
   > 
   > 1. I am wondering if it makes more sense to refactor `Range` to extend `RowRange`. At first glance it seems like the two should be able to share a lot of code as the main difference is comparing Text start/end row vs Key start/end otherwise the core methods/logic for the Range should be pretty similar. But again, I haven't looked in detail or thought through it all the way so maybe there are other reasons to keep it separate.
   > 2. Another option, which I like but may not work if we consider Range public API, is to refactor things and rename the existing Range object to KeyRange and have a more generic abstract Range object and then have `RowRange` and `KeyRange` extend that and share code. Having two descriptive objects of `RowRange` and `KeyRange` and have them share a common abstract class would be quite nice.
   
   It definitely seems like there should be some refactoring to share code. Of these two ideas the first seems like it would be a lot less disruptive as the Range API would not need to change. But the second idea seems maybe more complete(?) or optimal(?) (maybe how things would be done if creating both range and rowrange from scratch at this point) but more disruptive due to changing of the existing API. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org