Skip to content
This repository was archived by the owner on Jan 13, 2023. It is now read-only.

Upgrade complete implementation of LinkedList #7

Merged
merged 2 commits into from
Oct 21, 2018

Conversation

Podolian
Copy link
Contributor

No description provided.

@Podolian Podolian self-assigned this Oct 18, 2018
@Podolian Podolian requested a review from tboychuk October 18, 2018 10:05
@Podolian Podolian changed the base branch from master to exercise/completed October 18, 2018 17:27
@Podolian Podolian changed the title Exercise/completed - 6 Upgrade complete implementation of LinkedList Upgrade complete implementation of LinkedList Oct 18, 2018
}

private Node<E> findTail() {
Node<E> followingNode = Objects.requireNonNull(head);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you name it followingNode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I shouldn't name it so.
Changing that variable name to currentNode.

return LookForElement(element);
}

private boolean LookForElement(E element) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not the best name for method. It does not follow naming convention. In addition, it would be better to put this code into method contains()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, moving code to contains() method.


@Test
public void testAddElement() {
testLinkedList.add(7);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use BUILD OPERATE ASSERT pattern for test methods. E.g. you separate three parts of a test using empty line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying the pattern.

…ode`

- move `LookForElement()` code to method `contains()` and delete `LookForElement()` method
- apply BUILD OPERATE ASSERT pattern to `LinkedListTest`
@tboychuk tboychuk merged commit a78ddc3 into exercise/completed Oct 21, 2018
@tboychuk tboychuk deleted the exercise/completed-6 branch October 21, 2018 13:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants