Feature update - Add FDeque: Immutable Functional Double-Ended Queue and JUnit Tests #62
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces a new immutable, functional double-ended queue implementation through the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FDeque
Client->>FDeque: create()
FDeque-->>Client: new FDeque instance
Client->>FDeque: enqueFront(item)
FDeque-->>Client: new instance (updated front)
Client->>FDeque: enqueBack(item)
FDeque-->>Client: new instance (updated back)
Client->>FDeque: peekFront()/peekBack()
FDeque-->>Client: element value
Client->>FDeque: dequeFront()/dequeBack()
FDeque-->>Client: new instance (immutable update)
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/main/java/FDeque.java (5)
1-1: Remove unused import.The
ArrayListimport is not used in the implementation.-import java.util.ArrayList;
6-10: Add class-level documentation.Consider adding class-level JavaDoc to describe the purpose and behavior of this immutable double-ended queue implementation.
+/** + * An immutable double-ended queue implementation that maintains elements in two internal lists. + * All operations return new instances, preserving immutability. + * + * @param <T> the type of elements held in this deque + */ public final class FDeque<T> {
12-19: Add parameter validation in constructor.While the constructor is private, it's still good practice to validate parameters to catch potential issues early.
private FDeque(List<T> front, List<T> back) { + if (front == null || back == null) { + throw new IllegalArgumentException("Lists cannot be null"); + } this.front = Collections.unmodifiableList(front); this.back = Collections.unmodifiableList(back); }
47-60: Fix inconsistent indentation in dequeue methods.The indentation in both dequeue methods is inconsistent, making the code harder to read.
public FDeque<T> dequeFront() { if (front.isEmpty() && back.isEmpty()) { - return create(); } - else if (front.isEmpty()) { + return create(); + } else if (front.isEmpty()) { List<T> newBack = new LinkedList<>(back); newBack.removeFirst(); return new FDeque<>(Collections.emptyList(), newBack); } - else { + } else { List<T> newFront = new LinkedList<>(front); newFront.removeFirst(); return new FDeque<>(newFront, back); } } public FDeque<T> dequeBack() { if (back.isEmpty() && front.isEmpty()) { - return create(); - } else if (back.isEmpty()) { - List<T> newFront = new LinkedList<>(front); - newFront.removeLast(); - return new FDeque<>(Collections.emptyList(), newFront); - } - else { + return create(); + } else if (back.isEmpty()) { + List<T> newFront = new LinkedList<>(front); + newFront.removeLast(); + return new FDeque<>(Collections.emptyList(), newFront); + } else { List<T> newBack = new LinkedList<>(back); newBack.removeLast(); return new FDeque<>(front, newBack); } }Also applies to: 66-79
85-105: Consider using Optional for peek operations.Instead of returning null for empty deques, consider using Optional to make the absence of a value more explicit.
-public T peekFront() { +public Optional<T> peekFront() { if (!front.isEmpty()) { - return front.getFirst(); + return Optional.of(front.getFirst()); } else if (!back.isEmpty()) { - return back.getFirst(); + return Optional.of(back.getFirst()); } - return null; + return Optional.empty(); } -public T peekBack() { +public Optional<T> peekBack() { if (!back.isEmpty()) { - return back.getLast(); + return Optional.of(back.getLast()); } else if (!front.isEmpty()) { - return front.getLast(); + return Optional.of(front.getLast()); } - return null; + return Optional.empty(); }src/test/java/FDequeTest.java (2)
5-89: Enhance test coverage with additional scenarios.While the current tests are good, consider adding:
- Tests for null element handling
- Tests with different types (Integer, custom objects)
- Property-based testing for invariants
Example additional tests:
@Test void testNullElement() { FDeque<String> deque = FDeque.create(); assertThatThrownBy(() -> deque.enqueFront(null)) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> deque.enqueBack(null)) .isInstanceOf(IllegalArgumentException.class); } @Test void testWithInteger() { FDeque<Integer> deque = FDeque.create(); deque = deque.enqueFront(1).enqueBack(2); assertThat(deque.peekFront()).isEqualTo(1); assertThat(deque.peekBack()).isEqualTo(2); }
73-78: Add assertions for peek after empty deque operations.The test for empty deque operations should verify that peek operations still return null after dequeuing.
@Test void testPeekOnEmptyList() { FDeque<String> deque = FDeque.create(); assertThat(deque.peekFront()).isNull(); assertThat(deque.peekBack()).isNull(); + + // Verify peek still returns null after dequeue operations + deque = deque.dequeFront(); + assertThat(deque.peekFront()).isNull(); + assertThat(deque.peekBack()).isNull(); }
Summary by CodeRabbit