Skip to content
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.

Sign up for GitHub

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

list-ops: add to track #207

Merged
merged 13 commits into from
Feb 21, 2017
Merged

list-ops: add to track #207

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5a95ad7
dibs: I will implement list-ops
javaeeeee Dec 8, 2016
a6b7fe2
Merge branch 'master' into list-ops
javaeeeee Jan 9, 2017
c19aaa9
Added list-ops exercise to the Java track
javaeeeee Jan 9, 2017
1aaf04e
Updated repo to the latest version and resolved conflicts in config.json
javaeeeee Jan 9, 2017
bf2c51b
Removed NetBeans settings from the project dir
javaeeeee Jan 9, 2017
5371c79
Resolved conflicts in config.json
javaeeeee Jan 10, 2017
716536e
Updated test according to some changes required
javaeeeee Jan 12, 2017
de6eaa8
Implemented the requested changes
javaeeeee Jan 18, 2017
69cebae
Removed ranges for short lists
javaeeeee Jan 26, 2017
bc26145
Added collect() operation and tests
javaeeeee Jan 26, 2017
7bd1529
Made required changes
javaeeeee Feb 13, 2017
2ea76ab
Merge branch 'master' into list-ops
stkent Feb 18, 2017
af97e5a
Merge branch 'master' into list-ops
jtigger Feb 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 47 additions & 68 deletions exercises/list-ops/src/test/java/ListOpsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ public void lengthOfAnEmptyListShouldBeZero() {

@Test
@Ignore
public void shouldReturnCorrectLengthOfAnNonEmptyList() {
final int expected = 4;
public void shouldReturnTheCorrectLengthOfAnNonEmptyList() {
final List<Integer> list = Collections.unmodifiableList(
IntStream.range(0, expected)
IntStream.range(0, 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
.collect(Collectors.toList())
);
final int actual = ListOps.length(list);
final int expected = list.size();

assertEquals(expected, actual);
}
Expand All @@ -53,23 +53,22 @@ public void shouldReverseAnEmptyList() {

@Test
@Ignore
public void shouldReverseNonEmptyList() {
public void shouldReverseANonEmptyList() {
final int length = 100;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 extracting a value to a variable is a great move where the input and output need to agree.

🤔 this larger number forces us to use IntStream.range() to generate the list (and applying an operation on top of it; here sorting). If we start with a smaller value, we could start with the transformed value.

Example:

final List<Integer> list = Arrays.asList(0, 1, 2, 3, 4, 5, 6, 7, 8);
final List<Integer> reversed = Arrays.asList(8, 7, 6, 5, 4, 3, 2, 1, 0);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated code.

final int startValue = 0;
final List<Integer> reversedList
= IntStream.range(startValue, length)
.boxed()
.collect(Collectors.toList());
Collections.reverse(reversedList);
final List<Integer> expected
= Collections.unmodifiableList(reversedList);
final List<Integer> list = Collections.unmodifiableList(
IntStream.range(startValue, length)
.boxed()
.collect(Collectors.toList())
);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

final List<Integer> actual
= ListOps.reverse(list);
final List<Integer> expected = reversedList;
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 what do you think of using reversedList as the expected value directly in the assert? I'm thinking this could increase the readability of the assert itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed reversedList altogether.


assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand All @@ -78,7 +77,7 @@ public void shouldReverseNonEmptyList() {

@Test
@Ignore
public void mapOfAnEmptyListShouldBeAnEmptyList() {
public void shouldMapAnEmptyListAndReturnAnEmptyList() {
final List<Integer> actual = ListOps.map(EMPTY_LIST, x -> x + 1);

assertNotNull(actual);
Expand All @@ -88,11 +87,11 @@ public void mapOfAnEmptyListShouldBeAnEmptyList() {
@Test
@Ignore
public void shouldMapNonEmptyList() {
final List<Integer> expected
= Collections.unmodifiableList(Arrays.asList(2, 4, 6, 8));
final List<Integer> list
= Collections.unmodifiableList(Arrays.asList(1, 3, 5, 7));
final List<Integer> actual = ListOps.map(list, x -> x + 1);
final List<Integer> expected
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 (redundant suggestion) this "literal" (as close as we get in Java) is short enough that it could fit inline in the assert, below. The motivation, here, is to bring the declaration of the value as close to its use as possible (in this case, reducing that distance to zero).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated code.

= Arrays.asList(2, 4, 6, 8);

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand All @@ -101,7 +100,7 @@ public void shouldMapNonEmptyList() {

@Test
@Ignore
public void fileteredEmptyListShouldBeAnEmptyList() {
public void shouldFilterAnEmptyListanddReturnAnEmptyList() {
final List<Integer> actual = ListOps.filter(EMPTY_LIST, x -> x > 0);

assertNotNull(actual);
Expand All @@ -111,12 +110,11 @@ public void fileteredEmptyListShouldBeAnEmptyList() {
@Test
@Ignore
public void shouldFilterNonEmptyList() {
final List<Integer> expected
= Collections.unmodifiableList(Arrays.asList(1, 3));
final List<Integer> list = Collections.unmodifiableList(
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 I think it is really important that we take the opportunity to show-off the Java Streams API as much as we can, here; using the IntStream.range() idiom being one of those. But in this specific case, as written, it feels a little awkward to use Arrays.asList() for a two-element list and then upshift to IntStream to create a four-element list.

💡 you could create a large input list and use a highly-selective function in filter() that yields a short list. For example, a list from [1, 21) where values are evenly divisible by 5?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a longer list and introduced a filter. Extracted predicate for reuse.

IntStream.range(0, 4).boxed().collect(Collectors.toList())
);
final List<Integer> actual = ListOps.filter(list, x -> x % 2 > 0);
final List<Integer> expected = Arrays.asList(1, 3);

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand All @@ -137,18 +135,16 @@ public void shouldReturnAnEmptyListWhenAnEmptyListIsAppendedToAnEmptyList() {
@Test
@Ignore
public void shouldAppendAnEmptyListToANonEmptyList() {
final List<Integer> expected
= Collections.unmodifiableList(
IntStream.range(0, 4)
.boxed()
.collect(Collectors.toList())
);
final List<Integer> listTo
= IntStream.range(0, 4)
.boxed()
.collect(Collectors.toList());
assertFalse(ListOps.append(listTo, EMPTY_LIST));
final List<Integer> actual = listTo;
final List<Integer> expected
= IntStream.range(0, 4)
.boxed()
.collect(Collectors.toList());

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand All @@ -158,12 +154,6 @@ public void shouldAppendAnEmptyListToANonEmptyList() {
@Test
@Ignore
public void shouldAppendANonEmptyListToAnEmptyList() {
final List<Integer> expected
= Collections.unmodifiableList(
IntStream.range(0, 4)
.boxed()
.collect(Collectors.toList())
);
List<Integer> listTo = new ArrayList<>();
final List<Integer> listFrom
= Collections.unmodifiableList(
Expand All @@ -173,6 +163,10 @@ public void shouldAppendANonEmptyListToAnEmptyList() {
);
assertTrue(ListOps.append(listTo, listFrom));
final List<Integer> actual = listTo;
final List<Integer> expected
= IntStream.range(0, 4)
.boxed()
.collect(Collectors.toList());

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand All @@ -182,12 +176,6 @@ public void shouldAppendANonEmptyListToAnEmptyList() {
@Test
@Ignore
public void shouldAppendNonEmptyLists() {
final List<Integer> expected
= Collections.unmodifiableList(
IntStream.range(0, 8)
.boxed()
.collect(Collectors.toList())
);
final List<Integer> listTo
= IntStream.range(0, 4)
.boxed()
Expand All @@ -200,6 +188,10 @@ public void shouldAppendNonEmptyLists() {
);
assertTrue(ListOps.append(listTo, listFrom));
final List<Integer> actual = listTo;
final List<Integer> expected
= IntStream.range(0, 8)
.boxed()
.collect(Collectors.toList());

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand All @@ -218,19 +210,17 @@ public void shouldConcatenateZeroLists() {
@Test
@Ignore
public void shouldConcatenateOneNonEmptyList() {
final List<Integer> expected
= Collections.unmodifiableList(
IntStream.range(0, 4)
.boxed()
.collect(Collectors.toList())
);
final List<Integer> list
= Collections.unmodifiableList(
IntStream.range(0, 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
.collect(Collectors.toList())
);
final List<Integer> actual = ListOps.concat(list);
final List<Integer> expected
= IntStream.range(0, 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
.collect(Collectors.toList());

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand Down Expand Up @@ -258,19 +248,17 @@ public void shouldConcatenateTwoEmptyLists() {
@Test
@Ignore
public void shouldConcatenateOneEmptyAndOneNonEmptyLists() {
final List<Integer> expected
= Collections.unmodifiableList(
IntStream.range(0, 4)
.boxed()
.collect(Collectors.toList())
);
final List<Integer> list
= Collections.unmodifiableList(
IntStream.range(0, 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
.collect(Collectors.toList())
);
final List<Integer> actual = ListOps.concat(list, EMPTY_LIST);
final List<Integer> expected
= IntStream.range(0, 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
.collect(Collectors.toList());

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand All @@ -280,19 +268,17 @@ public void shouldConcatenateOneEmptyAndOneNonEmptyLists() {
@Test
@Ignore
public void shouldConcatenateOneNonEmptyAndOneEmptyLists() {
final List<Integer> expected
= Collections.unmodifiableList(
IntStream.range(0, 4)
.boxed()
.collect(Collectors.toList())
);
final List<Integer> list
= Collections.unmodifiableList(
IntStream.range(0, 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
.collect(Collectors.toList())
);
final List<Integer> actual = ListOps.concat(EMPTY_LIST, list);
final List<Integer> expected
= IntStream.range(0, 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
.collect(Collectors.toList());

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand All @@ -308,14 +294,10 @@ public void shouldConcatenateTwoListsWithSameElements() {
final List<Integer> list2 = Collections.unmodifiableList(
IntStream.range(1, 6).boxed().collect(Collectors.toList())
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

);
final List<Integer> sortedCancatenatedList
final List<Integer> expected
= Stream.concat(list1.stream(), list2.stream())
.collect(Collectors.toList());
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 ditto comment from above about putting the Java Streams API right next to the ListOps operation of the same name to introduce it. In fact, this one even more so since we're talking about concatenating Streams instead of Lists.

Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 expected can be feasibly expressed literally, here.

Collections.sort(sortedCancatenatedList);
final List<Integer> expected = Collections.unmodifiableList(
sortedCancatenatedList);
final List<Integer> actual = ListOps.concat(list1, list2);
Collections.sort(actual);

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand Down Expand Up @@ -343,9 +325,8 @@ public void shouldConcatenateSeveralLists() {
sortedCancatenatedList.addAll(list3);
sortedCancatenatedList.addAll(list4);
Collections.sort(sortedCancatenatedList);
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ I'm confused as to what's motivating the sort. I was expecting to see a few chunks that are, in series, appended. In fact, I'm expecting that the order is preserved. Makes me feel like I'm missing a subtle point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I see that it was unnecessary.

final List<Integer> expected = Collections.unmodifiableList(
sortedCancatenatedList
);
final List<Integer> expected
= sortedCancatenatedList;
final List<Integer> actual
= ListOps.concat(list1, list2, EMPTY_LIST, list3, list4);
Collections.sort(actual);
Expand Down Expand Up @@ -420,12 +401,7 @@ public void shouldReduceWithAnticommutativeAccumulator() {

@Test
@Ignore
public void reduceShouldConcatenateAnEmptyListAndANonEmptyList() {
final List<Integer> expected = Collections.unmodifiableList(
IntStream.range(0, 5)
.boxed()
.collect(Collectors.toList())
);
public void shouldReduceAnEmptyListAndANonEmptyListAndReturnConcatenation() {
final List<Integer> list = Collections.unmodifiableList(
IntStream.range(0, 5)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
Expand All @@ -436,6 +412,10 @@ public void reduceShouldConcatenateAnEmptyListAndANonEmptyList() {
new ArrayList<Integer>(),
accumulator,
combiner);
final List<Integer> expected
= IntStream.range(0, 5)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
.collect(Collectors.toList());

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand All @@ -444,12 +424,7 @@ public void reduceShouldConcatenateAnEmptyListAndANonEmptyList() {

@Test
@Ignore
public void reduceShouldConcatenateTwoNonEmptyLists() {
final List<Integer> expected = Collections.unmodifiableList(
IntStream.range(0, 10)
.boxed()
.collect(Collectors.toList())
);
public void shouldReduceTwoNonEmptyListsAndReturnConcatenation() {
final List<Integer> listOne = Collections.unmodifiableList(
IntStream.range(0, 5)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
Expand All @@ -465,6 +440,10 @@ public void reduceShouldConcatenateTwoNonEmptyLists() {
listOne,
accumulator,
combiner);
final List<Integer> expected
= IntStream.range(0, 10)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 use Arrays.asList(...) for short lists.

.boxed()
.collect(Collectors.toList());

assertNotNull(actual);
assertFalse(actual.isEmpty());
Expand Down