A factory and a test case to create RSSRead objects that read feeds off line (from a string containing the XML info)

[javierj]

This pull includes:

Factory class added to create RSSreader instances that read from a String instead of a real RSS feed. This class is for fast testing purpose.

New test case added to show how to use this factory.

A little refactor has been done in RSSReader::load for avoiding duplicate code.

Signed-off-by: JavierJ javierj@us.es

[javierj]

Dear Mr Horn.

Thank you very much for your notes. I have updated a new commit for this pulling request.

RSSReaderOffline has now three public constructors. First constructor accepts a string with the XML code, second one accepts a StringBuffer and the third one accepts an input stream. This class has no more public methods but the methods from RSSReader

There is no new code in RSSReader, only one refactorization to split the “load” method in two helper methods.

RSSReaderOffline is in a new subpackage, called “test”, to stress that this class is useful for writing test cases only.

There is also a new test package with two test cases. Test case RSSReaderOfflineTest, test the behavior of this class using its three constructor. This test case is also an example of how to write test cases using a RSSReaderOffline object.

I have seen that the test cases included in Android-RSS are written using JUnit4. Android SDK includes JUnit3. So I hae includes a version of the RSSReader test cases for JUnit 3 in this new test package. Now this test case may be executed in an Android JVM. Both test cases works properly.

I have changed the indentation of the code in RSSReader, RSSReaderIOffline and both test cases.

Previous RSSOfflineFactory (and its test case) have been dropped. No static dependences have been included.

Regards.

[ahorn]

There are some good bits in the patch. For example, it would be nice to integrate with the Android test harness. Do the tests with JUnit3 already work with Maven? To check, run "mvn test" on the command line.

Here are a few comments about the patch itself that need to be addressed:

• The rest of the projects only uses direct imports (e.g. not junit.framework.*)
• Please use proper indentation (e.g. [1] is making it difficult to see what blocks belong to what catches).
• All protected methods need to be documented (e.g. RSSReader::connectAndGetFeed()) to say what a subclassare expected and allowed to do, or any class invariances that they must respect
• Please base your commits of the latest ahorn/master branch (not javierj/master). This makes sure that there is a clean commit that shows others what has changed. Currently, these changes are with respect to your earlier commit. Instead, create a new commit from the original ahorn/android-rss repository.
• The patch introduces a potential resource leak bug [2] as part of the "refactoring" of RSSReader::load().

I can recommend the book: Effective Java: Second Edition.

[1] https://github.com/javierj/android-rss/blob/931b210b01a46184537dea895ba6132f81b85cd8/src/main/java/org/mcsoxford/rss/RSSReader.java#L106

[2] https://github.com/javierj/android-rss/commit/931b210b01a46184537dea895ba6132f81b85cd8#L0R131

[javierj]

Dear Mr. Horn.

Thank you for your valuable notes.

I don’t use Maven. I run the full test suite directly from my Eclipse. I think the android test cases included in the pull request should work with maven out-of-the-shell, due they extend junit.framewotk.TestCase and they don’t use anything from the Android API.

By the way, android-RSS uses the apache HTTP libraries which are not part of the Java SDK but they are included in the Android SDK.

I have fixed the generic imports and I have tried to fiz the tabulations of the source code. Please note that

Leak bug has been corrected closing the stream in RSSReader:: parseRSSFeed. The risk is very high when refactoring without a good test suite, even in simple refactors.

I’m not an expert using Git and Github. Github’s help indicate that you have to fork a repository, to make a commit to that repository and then, to request a pull request with the original repository to propose a change to the original Project.

Maybe I could close this pull request and open a new one with the final (so hope I ;) ) Anyway, I may work in any other way if you want to.

I’ll commit the last changes soon. Thank you very much.

[ahorn]

Hi @javierj

For some time now, I wanted to update the README with some contributor information, and references to projects/products that are using android-rss. If you are willing to share any of this information, please feel free to provide the following, or a subset thereof:

• Your name and/or organization you would like to be mentioned;
• The project name, or product, that is using android-rss.

Many thanks again for your contributions!