[GitHub] [geode] Kris-10-0 commented on a change in pull request #7389: GEODE-9950: Add LRANGE command

2022-02-25 Thread GitBox


Kris-10-0 commented on a change in pull request #7389:
URL: https://github.com/apache/geode/pull/7389#discussion_r815176258



##
File path: 
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
##
@@ -42,16 +45,65 @@ public RedisList() {
 this.elementList = new SizeableByteArrayList();
   }
 
+  /**
+   * @param start start index of desired elments
+   * @param stop stop index of desired elments
+   * @return list of elements in the range (inclusive).
+   */
+  public List lrange(int start, int stop) {
+start = transformNegIndexToPosIndex(start);
+stop = transformNegIndexToPosIndex(stop);
+
+// Out of range negative index changes to 0
+start = Math.max(0, start);
+
+int elementSize = elementList.size();
+if (start > stop || elementSize <= start) {
+  return Collections.emptyList();
+}
+
+// Out of range positive stop index changes to last index available
+stop = elementSize <= stop ? elementSize - 1 : stop;
+
+List result = new LinkedList<>();
+int curIndex;
+
+// Finds the shortest distance to access nodes in range
+if (start <= elementSize - stop - 1) {
+  // Starts at head to access nodes at start index then iterates forwards
+  curIndex = 0;
+  ListIterator iterator = elementList.listIterator(curIndex);
+
+  while (iterator.hasNext() && curIndex <= stop) {
+byte[] element = iterator.next();
+if (start <= curIndex) {
+  result.add(element);
+}
+curIndex = iterator.nextIndex();

Review comment:
   It looks like calling a normal iterator just returns a list iterator.




-- 
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-unsubscr...@geode.apache.org

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




[GitHub] [geode] Kris-10-0 commented on a change in pull request #7389: GEODE-9950: Add LRANGE command

2022-02-25 Thread GitBox


Kris-10-0 commented on a change in pull request #7389:
URL: https://github.com/apache/geode/pull/7389#discussion_r815167406



##
File path: 
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
##
@@ -42,16 +45,65 @@ public RedisList() {
 this.elementList = new SizeableByteArrayList();
   }
 
+  /**
+   * @param start start index of desired elments
+   * @param stop stop index of desired elments
+   * @return list of elements in the range (inclusive).
+   */
+  public List lrange(int start, int stop) {
+start = transformNegIndexToPosIndex(start);
+stop = transformNegIndexToPosIndex(stop);
+
+// Out of range negative index changes to 0
+start = Math.max(0, start);
+
+int elementSize = elementList.size();
+if (start > stop || elementSize <= start) {

Review comment:
   This checks if the start is greater than the element size. But I can 
move the normalization of stop before the check.




-- 
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-unsubscr...@geode.apache.org

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




[GitHub] [geode] Kris-10-0 commented on a change in pull request #7389: GEODE-9950: Add LRANGE command

2022-02-23 Thread GitBox


Kris-10-0 commented on a change in pull request #7389:
URL: https://github.com/apache/geode/pull/7389#discussion_r813440408



##
File path: 
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisList.java
##
@@ -42,16 +46,65 @@ public RedisList() {
 this.elementList = new SizeableByteArrayList();
   }
 
+  /**
+   * @param start start index of desired elments
+   * @param stop stop index of desired elments
+   * @return list of elements in the range (inclusive).
+   */
+  public List lrange(int start, int stop) {
+start = transformNegIndexToPosIndex(start);
+stop = transformNegIndexToPosIndex(stop);
+
+// Out of range negative index changes to 0
+start = start < 0 ? 0 : start;
+
+int elementSize = elementList.size();
+if (start > stop || elementSize <= start) {
+  return Collections.emptyList();
+}
+
+// Out of range positive stop index changes to last index available
+stop = elementSize <= stop ? elementSize - 1 : stop;
+
+List result = new LinkedList<>();
+int curIndex;
+
+// Finds the shortest distance to access nodes in range
+if (start <= elementSize - stop - 1) {
+  // Starts at head to access nodes at start index then iterates forwards
+  ListIterator iterator = elementList.listIterator();

Review comment:
   I changed it, but in what way does it create that small optimization? 
   
   I also modified the iterator for when it goes backwards since the default 
constructor for that is basically creating an listIterator with  an index that 
is the size of the list. It seemed similar to your suggestion here. 




-- 
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-unsubscr...@geode.apache.org

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