helun / Ektorp

Java API for CouchDB
Apache License 2.0
300 stars 146 forks source link

Ektorp Android - Can we have a standard interface for Builder? #145

Open yaronyg opened 11 years ago

yaronyg commented 11 years ago

I am writing code that needs to run both in Java and Android and I have to go through various weird contortions because AndroidHttpClient.Builder and HttpClient.Builder are unrelated classes. Life would be much happier if they both came from the same interface so I could pass a builder object in to a function who could configure it the same way regardless of it being called in Java or Android.

In fact the really happy solution would be if builder was added to the HttpClient definition and if the AndroidHttpClient class was automatically picked when the Android environment was detected so that the exact same code could be used both in Java and Android, only differing by which Ektorp JAR one pulled down. In other words I would always call some top level HttpClient class who would pick an implementation by detecting the local environment/JAR.

YannRobert commented 11 years ago

AndroidHttpClient class seems to be just a copy of StdHttpClient class, with just some minor differences.

Since StdHttpClient methods are now protected (they were private at the time AndroidHttpClient class was created), the AndroidHttpClient could be changed to just extend StdHttpClient and override appropriate methods.

Same applies to StdHttpClient.Builder and AndroidHttpClient.Builder

YannRobert commented 11 years ago

Would this fix the issue ? At least this solves a lot of code duplication.

yaronyg commented 11 years ago

tldr - This is an improvement on the current situation and worth doing! But we could do better.

Long Winded Version -

Just getting rid of the duplicate code is a wondrous enough accomplishment! :) And certainly this would be better than the current situation. But the ideal solution would make the user not care if they are on Android or Java. The user always calls HttpClient and HttpClient.Builder and it always works regardless of environment. This is actually doable in a way that would let us get rid of the special Android project.

The idea is that inside of HttpClient and HttpClient.Builder at the key points where we need different code for Android we could put in an environmental check to see which environment we are in. The only problem with this approach is that if Apache's HttpClient APIs break backwards compatibility with what Google has put into Android. If we want to cover that possibility then we need a factory class that returns HttpClient and HttpClient.Builder which can pick what object to return (e.g. the Java flavor of HttpClient or the Android one) based on environment. That would let us completely separate out the code in a way that could be build differently for Android than for normal Java.

But my feeling is that the best experience is that the programmer doesn't need to care which environment we are in and having AndroidHttpClient and HttpClient doesn't achieve that.

Nevertheless I think this pull request is worth accepting since it makes the situation better and shouldn't do any harm.