Skip to content

Binary search #186

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

Merged
merged 9 commits into from
Dec 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion config.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@
"robot-simulator",
"bracket-push",
"pythagorean-triplet",
"binary-search-tree"
"binary-search-tree",
"binary-search"
],
"exercises": [
{
Expand Down Expand Up @@ -299,6 +300,11 @@
"slug": "binary-search-tree",
"difficulty": 1,
"topics": []
},
{
"slug": "binary-search",
"difficulty": 1,
"topics": []
}
],
"deprecated": [
Expand Down
17 changes: 17 additions & 0 deletions exercises/binary-search/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
apply plugin: "java"
apply plugin: "eclipse"
apply plugin: "idea"

repositories {
mavenCentral()
}

dependencies {
testCompile "junit:junit:4.12"
}
test {
testLogging {
exceptionFormat = 'full'
events = ["passed", "failed", "skipped"]
}
}
40 changes: 40 additions & 0 deletions exercises/binary-search/src/example/java/BinarySearch.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@

import java.util.List;

public class BinarySearch<T extends Comparable<T>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticed that this generic type means the example implementation is a lot more general than the tests require. @exercism/java is this a good feature in a reference solution, or does it veer towards YAGNI territory?

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I currently give wide berth to implementors in what happens in the example. As long as it passes the tests and represents a true solution, it's generally been implementor's choice.

I think this way because practitioners don't see these. The high value conversations in PRs are about how the test suite flows, the API decisions therein and whether we're implementing the exercise correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, and thank you for your comments. Here is a question: Do we need .keep files? Spotted that some were removed in one of the commits.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since there are other files in exercises/binary-search/src/main/java/ and exercises/binary-search/src/test/java/, it should be safe to remove exercises/binary-search/src/main/java/.keep and exercises/binary-search/src/test/java/.keep. Thanks for checking!


private List<T> array;
private int arraySize;

public BinarySearch(List<T> array) {
this.array = array;
this.arraySize = array.size();
}

public int indexOf(T value) {
return search(value);
}

public List<T> getArray() {
return array;
}

private int search(T value) {
int left = 0;
int right = this.arraySize - 1;
int middle;
T element;
while (left <= right) {
middle = (int) Math.floor(0.5 * (left + right));
element = this.array.get(middle);
if (value.compareTo(element) > 0) {
left = middle + 1;
} else if (value.compareTo(element) < 0) {
right = middle - 1;
} else {
return middle;
}
}
return -1;
}
}
Empty file.
4 changes: 4 additions & 0 deletions exercises/binary-search/src/main/java/BinarySearch.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

public class BinarySearch {

}
Empty file.
136 changes: 136 additions & 0 deletions exercises/binary-search/src/test/java/BinarySearchTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import static org.junit.Assert.assertEquals;
import org.junit.Ignore;
import org.junit.Test;

public class BinarySearchTest {

public static final List<Integer> EMPTY_LIST
= Collections.unmodifiableList(new ArrayList<Integer>(0));

public static final List<Integer> LIST_OF_UNIT_LENGTH
= Collections.unmodifiableList(
Arrays.asList(6)
);

private static final List<Integer> SORTED_LIST
= Collections.unmodifiableList(
Arrays.asList(1, 3, 4, 6, 8, 9, 11)
);

public static final List<Integer> SORTED_LIST_OF_ODD_LENGTH
= Collections.unmodifiableList(
Arrays.asList(1, 3, 5, 8, 13, 21, 34, 55,
89, 144, 233, 377, 634)
);

public static final List<Integer> SORTED_LIST_OF_EVEN_LENGTH
= Collections.unmodifiableList(
Arrays.asList(1, 3, 5, 8, 13, 21, 34, 55,
89, 144, 233, 377)
);

@Test
public void findsAValueInAnArrayWithOneElement() {
BinarySearch<Integer> sut = new BinarySearch<>(LIST_OF_UNIT_LENGTH);
final int value = 6;
final int actual = sut.indexOf(value);
final int expected = 0;
assertEquals(expected, actual);
}

@Ignore
@Test
public void findsAValueInTheMiddleOfAnArray() {
BinarySearch<Integer> sut = new BinarySearch<>(SORTED_LIST);
final int value = 6;
final int actual = sut.indexOf(value);
final int expected = 3;
assertEquals(expected, actual);
}

@Ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for ignoring all but the first test by default! Implementors thank you.

@Test
public void findsAValueAtTheBeginningOfAnArray() {
BinarySearch<Integer> sut = new BinarySearch<>(SORTED_LIST);
final int value = 1;
final int actual = sut.indexOf(value);
final int expected = 0;
assertEquals(expected, actual);
}

@Ignore
@Test
public void findsAValueAtTheEndOfAnArray() {
BinarySearch<Integer> sut = new BinarySearch<>(SORTED_LIST);
final int value = 11;
final int actual = sut.indexOf(value);
final int expected = 6;
assertEquals(expected, actual);
}

@Ignore
@Test
public void findsAValueInAnArrayOfOddLength() {
BinarySearch<Integer> sut = new BinarySearch<>(SORTED_LIST_OF_ODD_LENGTH);
final int value = 144;
final int actual = sut.indexOf(value);
final int expected = 9;
assertEquals(expected, actual);
}

@Ignore
@Test
public void findsAValueInAnArrayOfEvenLength() {
BinarySearch<Integer> sut
= new BinarySearch<>(SORTED_LIST_OF_EVEN_LENGTH);
final int value = 21;
final int actual = sut.indexOf(value);
final int expected = 5;
assertEquals(expected, actual);
}

@Ignore
@Test
public void identifiesThatAValueIsNotIncludedInTheArray() {
BinarySearch<Integer> sut = new BinarySearch<>(SORTED_LIST);
final int value = 7;
final int actual = sut.indexOf(value);
final int expected = -1;
assertEquals(expected, actual);
}

@Ignore
@Test
public void aValueSmallerThanTheArraysSmallestValueIsNotIncluded() {
BinarySearch<Integer> sut = new BinarySearch<>(SORTED_LIST);
final int value = 0;
final int actual = sut.indexOf(value);
final int expected = -1;
assertEquals(expected, actual);
}

@Ignore
@Test
public void aValueLargerThanTheArraysSmallestValueIsNotIncluded() {
BinarySearch<Integer> sut = new BinarySearch<>(SORTED_LIST);
final int value = 13;
final int actual = sut.indexOf(value);
final int expected = -1;
assertEquals(expected, actual);
}

@Ignore
@Test
public void nothingIsIncludedInAnEmptyArray() {
BinarySearch<Integer> sut = new BinarySearch<>(EMPTY_LIST);
final int value = 1;
final int actual = sut.indexOf(value);
final int expected = -1;
assertEquals(expected, actual);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The binary-search problem has associated canonical test data located here.

In general, when implementing a new exercise in a track we should reference those canonical test cases (if they exist) to ensure consistency across languages. I think you have most of the expected cases covered, but for future maintainability it would be awesome if you could adjust your tests to match the canonical naming/data exactly. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. Thank you for the reference.

}