From 69d53cf33f30fa2005769c3845570570b6e99e95 Mon Sep 17 00:00:00 2001 From: Xavi Rigau Date: Wed, 23 Dec 2015 12:48:46 +0100 Subject: [PATCH 1/4] Adds `popResult` methods so that we can choose to not cache results - Added more jdoc and tests --- .../sexp/finder/BasicElementFinder.java | 13 ++++++++ .../com/novoda/sexp/finder/ElementFinder.java | 22 +++++++++++++ .../novoda/sexp/finder/ListElementFinder.java | 10 ++++++ .../sexp/finder/BasicElementFinderShould.java | 33 ++++++++++++++++++- .../sexp/finder/ListElementFinderShould.java | 15 +++++++++ 5 files changed, 92 insertions(+), 1 deletion(-) diff --git a/sexp/src/main/java/com/novoda/sexp/finder/BasicElementFinder.java b/sexp/src/main/java/com/novoda/sexp/finder/BasicElementFinder.java index 9045fd7..934908d 100644 --- a/sexp/src/main/java/com/novoda/sexp/finder/BasicElementFinder.java +++ b/sexp/src/main/java/com/novoda/sexp/finder/BasicElementFinder.java @@ -39,6 +39,19 @@ public T getResultOrThrow() { return result; } + @Override + public T popResult() { + T r = result; + result = null; + return r; + } + + @Override + public T popResultOrThrow() { + validateResult(); + return popResult(); + } + private void validateResult() { if (result == null) { throw new ResultNotFoundException(); diff --git a/sexp/src/main/java/com/novoda/sexp/finder/ElementFinder.java b/sexp/src/main/java/com/novoda/sexp/finder/ElementFinder.java index 62348e6..edcdbbd 100644 --- a/sexp/src/main/java/com/novoda/sexp/finder/ElementFinder.java +++ b/sexp/src/main/java/com/novoda/sexp/finder/ElementFinder.java @@ -25,7 +25,29 @@ public interface ElementFinder extends ParseWatcher { @Override void onParsed(R item); + /** + * @return the result of the parsing if the parsing was completed and tag was found + * or null if the tag was not found or parsing didn't complete. + */ R getResult(); + /** + * @return the result of the parsing if the parsing was completed and tag was found + * or throws an exception if the tag was not found or parsing didn't complete. + */ R getResultOrThrow(); + + /** + * Same as {@link #getResult} but clears the result instead of keeping it. + * @return the result of the parsing if the parsing was completed and tag was found + * or null if the tag was not found or parsing didn't complete. + */ + R popResult(); + + /** + * Same as {@link #getResultOrThrow} but clears the result instead of keeping it. + * @return the result of the parsing if the parsing was completed and tag was found + * or throws an exception if the tag was not found or parsing didn't complete. + */ + R popResultOrThrow(); } diff --git a/sexp/src/main/java/com/novoda/sexp/finder/ListElementFinder.java b/sexp/src/main/java/com/novoda/sexp/finder/ListElementFinder.java index 30d9569..cadc069 100644 --- a/sexp/src/main/java/com/novoda/sexp/finder/ListElementFinder.java +++ b/sexp/src/main/java/com/novoda/sexp/finder/ListElementFinder.java @@ -39,4 +39,14 @@ public T getResultOrThrow() { throw new UnsupportedOperationException("Has a listener to pass each item as parsed, so there is no result."); } + @Override + public T popResult() { + throw new UnsupportedOperationException("Has a listener to pass each item as parsed, so there is no result."); + } + + @Override + public T popResultOrThrow() { + throw new UnsupportedOperationException("Has a listener to pass each item as parsed, so there is no result."); + } + } diff --git a/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java b/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java index 5e41fe2..c10eb59 100644 --- a/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java +++ b/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java @@ -52,7 +52,7 @@ public void create_a_result_when_parsing_finished() throws Exception { elementCreator.onParsed(result); - assertThat(elementCreator.getResultOrThrow()).isEqualTo(result); + assertThat(elementCreator.getResult()).isEqualTo(result); } @Test(expected = BasicElementFinder.ResultNotFoundException.class) @@ -67,4 +67,35 @@ public void allow_null_results_when_get_result_is_used() throws Exception { assertThat(result).isNull(); } + @Test + public void pop_a_result_when_parsing_finished() throws Exception { + String result = "result"; + + elementCreator.onParsed(result); + + assertThat(elementCreator.popResult()).isEqualTo(result); + } + + @Test(expected = BasicElementFinder.ResultNotFoundException.class) + public void throw_exception_when_result_has_not_been_parsed_and_a_result_is_popped() throws Exception { + elementCreator.popResultOrThrow(); + } + + @Test + public void allow_null_results_when_get_result_is_popped() throws Exception { + Object result = elementCreator.popResult(); + + assertThat(result).isNull(); + } + + @Test + public void not_cache_result_after_a_result_is_popped() throws Exception { + String result = "result"; + + elementCreator.onParsed(result); + + assertThat(elementCreator.popResult()).isEqualTo(result); + assertThat(elementCreator.popResult()).isNull(); + } + } diff --git a/sexp/src/test/java/com/novoda/sexp/finder/ListElementFinderShould.java b/sexp/src/test/java/com/novoda/sexp/finder/ListElementFinderShould.java index c90d2cd..70df986 100644 --- a/sexp/src/test/java/com/novoda/sexp/finder/ListElementFinderShould.java +++ b/sexp/src/test/java/com/novoda/sexp/finder/ListElementFinderShould.java @@ -49,7 +49,22 @@ public void callWatcher_afterEachItemForTheListIsParsed() throws Exception { @Test(expected = UnsupportedOperationException.class) public void notSupportGetResult_asListCreatorWillCallbackAfterEveryListItemIsParsed() throws Exception { + elementCreator.getResult(); + } + + @Test(expected = UnsupportedOperationException.class) + public void notSupportGetResultOrThrow_asListCreatorWillCallbackAfterEveryListItemIsParsed() throws Exception { elementCreator.getResultOrThrow(); } + @Test(expected = UnsupportedOperationException.class) + public void notSupportPopResult_asListCreatorWillCallbackAfterEveryListItemIsParsed() throws Exception { + elementCreator.popResult(); + } + + @Test(expected = UnsupportedOperationException.class) + public void notSupportPopResultOrThrow_asListCreatorWillCallbackAfterEveryListItemIsParsed() throws Exception { + elementCreator.popResultOrThrow(); + } + } From 46a259e22400e14dfd86854b8a2db47a6bf01587 Mon Sep 17 00:00:00 2001 From: Xavi Rigau Date: Wed, 23 Dec 2015 12:53:36 +0100 Subject: [PATCH 2/4] sick codez by @fourlastor --- .../java/com/novoda/sexp/finder/BasicElementFinder.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sexp/src/main/java/com/novoda/sexp/finder/BasicElementFinder.java b/sexp/src/main/java/com/novoda/sexp/finder/BasicElementFinder.java index 934908d..ade5aa9 100644 --- a/sexp/src/main/java/com/novoda/sexp/finder/BasicElementFinder.java +++ b/sexp/src/main/java/com/novoda/sexp/finder/BasicElementFinder.java @@ -41,9 +41,11 @@ public T getResultOrThrow() { @Override public T popResult() { - T r = result; - result = null; - return r; + try { + return result; + } finally { + result = null; + } } @Override From 0141704886f4f165f40f92a444df4bfe6176052a Mon Sep 17 00:00:00 2001 From: Xavi Rigau Date: Wed, 23 Dec 2015 12:57:28 +0100 Subject: [PATCH 3/4] Better test names --- .../java/com/novoda/sexp/finder/BasicElementFinderShould.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java b/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java index c10eb59..d5359b4 100644 --- a/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java +++ b/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java @@ -56,7 +56,7 @@ public void create_a_result_when_parsing_finished() throws Exception { } @Test(expected = BasicElementFinder.ResultNotFoundException.class) - public void throw_exception_when_result_has_not_been_parsed_and_a_required_result_is_asked_for() throws Exception { + public void throw_exception_when_result_has_not_been_parsed_or_found_and_a_required_result_is_asked_for() throws Exception { elementCreator.getResultOrThrow(); } @@ -77,7 +77,7 @@ public void pop_a_result_when_parsing_finished() throws Exception { } @Test(expected = BasicElementFinder.ResultNotFoundException.class) - public void throw_exception_when_result_has_not_been_parsed_and_a_result_is_popped() throws Exception { + public void throw_exception_when_result_has_not_been_parsed_or_found_and_a_result_is_popped() throws Exception { elementCreator.popResultOrThrow(); } From c2d7bfb8f400ed635ab24adb69eae77de5ca5e16 Mon Sep 17 00:00:00 2001 From: Xavi Rigau Date: Wed, 23 Dec 2015 13:12:54 +0100 Subject: [PATCH 4/4] Reformats tests to follow given-when-then and removes unnecessary assert --- .../sexp/finder/BasicElementFinderShould.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java b/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java index d5359b4..0c33d04 100644 --- a/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java +++ b/sexp/src/test/java/com/novoda/sexp/finder/BasicElementFinderShould.java @@ -70,10 +70,11 @@ public void allow_null_results_when_get_result_is_used() throws Exception { @Test public void pop_a_result_when_parsing_finished() throws Exception { String result = "result"; - elementCreator.onParsed(result); - assertThat(elementCreator.popResult()).isEqualTo(result); + Object actual = elementCreator.popResult(); + + assertThat(actual).isEqualTo(result); } @Test(expected = BasicElementFinder.ResultNotFoundException.class) @@ -90,12 +91,12 @@ public void allow_null_results_when_get_result_is_popped() throws Exception { @Test public void not_cache_result_after_a_result_is_popped() throws Exception { - String result = "result"; + elementCreator.onParsed("ignore"); + elementCreator.popResult(); - elementCreator.onParsed(result); + Object actual = elementCreator.popResult(); - assertThat(elementCreator.popResult()).isEqualTo(result); - assertThat(elementCreator.popResult()).isNull(); + assertThat(actual).isNull(); } }