-
Notifications
You must be signed in to change notification settings - Fork 11
Draft: 48 extend persistent data structures with deque #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 21 commits
eda35d3
4f14869
4690c40
d591655
f186772
a28145e
b5a5810
afb6bca
cd9b78f
4cec676
75c8153
7d14d39
0a981a7
961b774
7234cb6
c4604d4
9be3445
e09ac80
30e0c25
7c55bf8
15f7b96
b7a8a7f
f5d4840
0e4dfd7
c668e2c
8d9b31d
ca2f3ff
153c25b
9bceebb
befc5a1
58be67a
191e30c
3317bc4
d11eaa8
25ae5c5
eb9ccbf
94dd4e9
5dce0db
f34e96d
8723ee4
31267c8
bb00c3d
8420b86
8934375
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,3 +37,4 @@ | |
| /lib/java | ||
| /lib/java-contrib | ||
|
|
||
| /.idea/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| // This file is part of SoSy-Lab Common, | ||
| // a library of useful utilities: | ||
| // https://github.com/sosy-lab/java-common-lib | ||
| // | ||
| // SPDX-FileCopyrightText: 2026 Dirk Beyer <https://www.sosy-lab.org> | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package org.sosy_lab.common.collect; | ||
|
|
||
| import com.google.errorprone.annotations.Immutable; | ||
| import com.google.errorprone.annotations.Var; | ||
| import java.util.Iterator; | ||
| import javax.annotation.Nullable; | ||
|
|
||
| /** | ||
| * A persistent implementation of a deque on the basis of {@link PersistentLinkedList}. | ||
| * | ||
| * <p>To avoid O(n) runtime complexity when accessing the bottom of the deque, two separate | ||
| * {@link PersistentLinkedList}s are used. {@code top} represents the top part of the deque, | ||
| * while {@code bottom} forms the lower part of the deque. If one were to reverse {@code bottom} | ||
| * and then add it to the bottom of {@code top}, one would receive one list representing the | ||
| * whole deque in correct order. | ||
| * | ||
| * <p>It provides operations to show the top- and bottom-most elements of the deque, as well as | ||
| * ones | ||
| * to remove them or add new items to the deque in either places. | ||
| * In most cases, these will complete in O(1). Occasionally, these operations will require more | ||
| * time, as the deque might need to be rebalanced (i.e. when one of the lists becomes empty, the | ||
| * other list is split up into top and bottom to further guarantee access to both ends of | ||
| * the deque). | ||
| * | ||
| * <p>Currently, it is only possible to create an empty deque and then add new elements one | ||
| * at a | ||
| * time. | ||
| * | ||
| * @param <T> type of elements to be stored in deque | ||
| */ | ||
| @Immutable(containerOf = "T") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The class is declared to be immutable, is actually immutable, but it is still using/allowing mutable methods.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need to list methods that are just handed through, like Overall this looks much better! Thank you! |
||
| public final class PersistentDeque<T> implements PersistentDequeInterface<T> { | ||
|
baierd marked this conversation as resolved.
Outdated
|
||
| final PersistentLinkedList<T> top; | ||
| final PersistentLinkedList<T> bottom; | ||
|
|
||
| public PersistentDeque() { | ||
| top = PersistentLinkedList.of(); | ||
| bottom = PersistentLinkedList.of(); | ||
| } | ||
|
|
||
| private PersistentDeque(PersistentLinkedList<T> top, PersistentLinkedList<T> bottom) { | ||
| this.top = top; | ||
| this.bottom = bottom; | ||
| } | ||
|
|
||
| /** | ||
| * Checks both sublists and returns true if both are empty, false if at least one is not. | ||
| * | ||
| * @return true if {@code top} and {@code bottom} are both empty, false if at least one is not | ||
| */ | ||
| @Override | ||
| public boolean isEmpty() { | ||
| return top.isEmpty() && bottom.isEmpty(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns element at the top of the deque. | ||
| * | ||
| * @return element at top of deque; null if deque empty | ||
| */ | ||
| @Nullable | ||
| @Override | ||
| public T getTop() { | ||
| //If deque contains only one element, one of the lists will be empty. Calling head() on an | ||
| // empty list throws an exception, so this needs to be caught. Further, the one element in | ||
| // the non-empty list should be returned, as it is both head and tail of the deque. | ||
| try { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your two try-catch blocks could be simplified by using only a single one.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Control-flow with exceptions should be avoided as much as possible (it is slow, hard to read/debug etc.). Why don't you simply check whether top/bottom are empty before calling There are multiple occurrences of this behavior in this class.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, they could have been avoided altogether. I've removed them all and altered the methods accordingly. |
||
| return top.head(); | ||
| } catch(IllegalStateException e1) { | ||
| try { | ||
| return bottom.head(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure you want the head here in all cases?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was a bit iffy about that too, but other options seemed bulky. As long as the rebalancing works correctly and is called where necessary, bottom should only contain at most one element if top.head() threw an exception. I will admit though that this logic relies on the rest of the code being sound, or it could cause mistakes...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. I'll take a closer look at the re-balancing once the formalities/API renamings etc. are done. |
||
| } catch(IllegalStateException e2) { | ||
| return null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you really want to return Try to look at this from a user-perspective; you don't know about the implementation details of a data-structure that you use. You just expect the guarantees given in the documentation. Suddenly you get an unexpected You can take a look at how we handle If an exception is thrown by a used method, it makes sense to just declare the using method as also throwing as long as you can not guarantee that your implementation does never throw this exception. Your documentation should be extended by appropriate reasoning if you also throw the exception (i.e. look at why and when the used method throws) What might have added to the confusion is that our The same applies to
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be resolved as all methods returning null do so because they were specified that way in java.util.Deque. |
||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns element at the bottom of the deque. | ||
| * | ||
| * @return element at bottom of deque; null if deque empty | ||
| */ | ||
| @Nullable | ||
| @Override | ||
| public T getBottom() { | ||
| //If deque contains only one element, one of the lists will be empty. Calling head() on an | ||
| // empty list throws an exception, so this needs to be caught. Further, the one element in | ||
| // the non-empty list should be returned, as it is both head and tail of the deque. | ||
| try { | ||
| return bottom.head(); | ||
| } catch(IllegalStateException e1) { | ||
| try { | ||
| return top.head(); | ||
| } catch(IllegalStateException e2) { | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Places new element on top of the deque. | ||
| * | ||
| * @param value element to be added to deque | ||
| * @return deque instance with new element added on top | ||
| */ | ||
| @Override | ||
| public PersistentDeque<T> insertTop(T value) { | ||
| return new PersistentDeque<>(top.with(value), bottom); | ||
| } | ||
|
|
||
| /** | ||
| * Places new element at the bottom of the deque. | ||
| * | ||
| * @param value element to be added to deque | ||
| * @return deque instance with new element added at the bottom | ||
| */ | ||
| @Override | ||
| public PersistentDeque<T> insertBottom(T value) { | ||
| return new PersistentDeque<>(top, bottom.with(value)); | ||
| } | ||
|
|
||
| /** | ||
| * Removes element at the top of the deque from deque. | ||
| * | ||
| * @return deque instance after top element has been removed | ||
| */ | ||
| @Override | ||
| public PersistentDeque<T> deleteTop() { | ||
| return new PersistentDeque<>(top.tail(), bottom).rebalanceDeque(); | ||
| } | ||
|
|
||
| /** | ||
| * Removes element at the bottom of the deque from deque. | ||
| * | ||
| * @return deque instance after bottom element has been removed | ||
| */ | ||
| @Override | ||
| public PersistentDeque<T> deleteBottom() { | ||
| return new PersistentDeque<>(top, bottom.tail()).rebalanceDeque(); | ||
| } | ||
|
|
||
| private PersistentDeque<T> rebalanceDeque() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes sense to document the invariant that has to hold for this method and for the |
||
| boolean topEmpty = top.isEmpty(); | ||
| boolean bottomEmpty = bottom.isEmpty(); | ||
|
|
||
| if (topEmpty && bottomEmpty) { | ||
| return this; | ||
| } else if (topEmpty && !bottomEmpty) { | ||
| return split(bottom.reversed()); | ||
| } else if (!topEmpty && bottomEmpty) { | ||
| return split(top); | ||
| } | ||
|
|
||
| return this; | ||
| } | ||
|
|
||
| private PersistentDeque<T> split(PersistentLinkedList<T> list) { | ||
| int size = list.size(); | ||
| int halfSize = size / 2; | ||
|
|
||
| if (size <= 0) { | ||
| throw new IllegalArgumentException("Cannot split empty list!"); | ||
| } else if (size == 1) { | ||
| return this; | ||
| } | ||
|
|
||
| @Var PersistentLinkedList<T> newTop = PersistentLinkedList.of(); | ||
| @Var PersistentLinkedList<T> newBottom = PersistentLinkedList.of(); | ||
| Iterator<T> iterator = list.iterator(); | ||
|
|
||
| for (int i = 0; i < size; i++) { | ||
| T element = iterator.next(); | ||
| if (i < halfSize) { | ||
| newTop = newTop.with(element); | ||
| } else { | ||
| newBottom = newBottom.with(element); | ||
| } | ||
| } | ||
| newTop = newTop.reversed(); | ||
| return new PersistentDeque<>(newTop, newBottom); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| // This file is part of SoSy-Lab Common, | ||
| // a library of useful utilities: | ||
| // https://github.com/sosy-lab/java-common-lib | ||
| // | ||
| // SPDX-FileCopyrightText: 2026 Dirk Beyer <https://www.sosy-lab.org> | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package org.sosy_lab.common.collect; | ||
|
|
||
| import com.google.errorprone.annotations.Immutable; | ||
|
|
||
| /** | ||
| * Interface for persistent deques. A persistent data structure is immutable, but provides cheap | ||
| * * copy-and-write operations. Thus, all write operations | ||
| * ({@link #insertTop(Object)}, {@link #insertBottom(Object)}, {@link #deleteTop()}, | ||
| * {@link #deleteBottom()}) will | ||
| * not modify the current | ||
| * instance, | ||
| * but return a new instance instead. | ||
| * | ||
| * @param <T> type of elements stored in deque | ||
| */ | ||
| @Immutable(containerOf = "T") | ||
| public interface PersistentDequeInterface<T> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can just inherit the proper Java interface instead of defining everything on your own. It is also a good idea to split the ideas of immutability and persistence, as it allows us to build immutable and also persistent data-structures independently of each other, but also helps understanding what each interface does, by splitting the concerns. Normally we would disallow certain methods due to immutability right away using a default implementation in an immutable interface (e.g. add and remove methods) . But we are still in Java 11 here (update to 17 is currently being worked on). So the solution is an abstract class instead. You can take a look at our AbstractImmutableMap for example. You also get a notion on how to use the annotations like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You actually had a reasoning why you did not import Note: you actually should support at least the core methods/concepts in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if I included in my reasoning that some of the methods required return types that I didn't think matched up with what would make sense for a persistent version. I.e. pop returns the element that was removed, which would leave no way to return the new, updated instance of the deque. Is that one of the cases where you would like it to throw an exception and create a different method that actually works instead?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is why we throw for all modifying methods in immutable data-structures. In your version you have the worst of both worlds, they look like they are mutable, but are not. Usually we then introduce new methods following the copy-then-modify approach, for example
baierd marked this conversation as resolved.
Outdated
|
||
|
|
||
| /** | ||
| * Returns true if deque is empty, false if not. | ||
| * | ||
| * @return true if deque empty, else false | ||
| */ | ||
| boolean isEmpty(); | ||
|
|
||
| /** | ||
| * Retrieves element at top of deque. | ||
| * | ||
| * @return element at top of deque | ||
| */ | ||
| T getTop(); | ||
|
|
||
| /** | ||
| * Retrieves element at bottom of deque. | ||
| * | ||
| * @return element at bottom of deque | ||
| */ | ||
| T getBottom(); | ||
|
|
||
| /** | ||
| * Inserts element at top of deque. | ||
| * | ||
| * @param value element to be inserted | ||
| * @return deque instance with new element at top of deque | ||
| */ | ||
| PersistentDeque<T> insertTop(T value); | ||
|
|
||
| /** | ||
| * Inserts element at bottom of deque. | ||
| * | ||
| * @param value element to be inserted | ||
| * @return deque instance with new element at bottom of deque | ||
| */ | ||
| PersistentDeque<T> insertBottom(T value); | ||
|
|
||
| /** | ||
| * Deletes element currently at top of deque. | ||
| * | ||
| * @return deque instance after top element has been removed | ||
| */ | ||
| PersistentDeque<T> deleteTop(); | ||
|
|
||
| /** | ||
| * Deletes element currently at bottom of deque. | ||
| * | ||
| * @return deque instance after bottom element has been removed | ||
| */ | ||
| PersistentDeque<T> deleteBottom(); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.