search

cybre-finn / next-companion

Rogue client for nextbike
GNU General Public License v3.0
23 stars 8 forks source link

Use a common HTTP-Client implementation & JSON-Libary #19

Open lgehr opened 4 years ago

lgehr commented 4 years ago

If I understand the code correct API-Calls to nextbike.com are currently handled by the self written RequestHandler Class.

I suggest to change to a common used general implementation like Retrofit. This would make the code more robust and it would easier for people to contribute. (e.g. me :]). Also I think #13 could be implemented very straightforward.

We would maybe need to change the Json-Libary from json.org to FasterXML's Jackson because I do not know if retrofit works with the json.org-libaries. Also json.org has the reputation to be quite slow.

What do you think @h0chi? I would be willing to implement the change. Maybe starting with LoginActivity.java to see if it fits the project. I would also be open to other implementations. I just suggested retrofit and jackson because I have some experience with them.

Also thank you for starting this project. I use the app everyday. :)

cybre-finn commented 4 years ago

Thank you for contributing!

I suggest to change to a common used general implementation like Retrofit. This would make the code more robust and it would easier for people to contribute. (e.g. me :]). Also I think #13 could be implemented very straightforward.

We would maybe need to change the Json-Libary from json.org to FasterXML's Jackson because I do not know if retrofit works with the json.org-libaries. Also json.org has the reputation to be quite slow.

I'd really like to stick to de facto standard implementations for this project. I heard of Retrofit and it seems to match. On the first sight, FasterXML does not, but I don't really care if Retrofit only works with FasterXML.

What do you think @h0chi? I would be willing to implement the change. Maybe starting with LoginActivity.java to see if it fits the project. I would also be open to other implementations. I just suggested retrofit and jackson because I have some experience with them.

I actually wrote this code years ago, before I enrolled into computer science and were able to write good code. At the moment I actually don't have time for active maintainance, but I really appreciate code contribution and will review and merge. Also looking for a co maintainer if you'd like to stick around.

When working on your patch also note this repo I started when dissecting the nextbike api: https://github.com/h0chi/nextbike-api-reverse-engineering

Something completely different: Is the app actually working for you? Few hours after I received your issue, I (unrelated) tried to rent a bike, didn't work and later that day nextbike sent me a mail that my "account has been locked due to payment method bla bla".

lgehr commented 4 years ago

I'd really like to stick to de facto standard implementations for this project. I heard of Retrofit and it seems to match. On the first sight, FasterXML does not, but I don't really care if Retrofit only works with FasterXML. Actually Retrofit seems to work with a range of Converters. It says on the website https://square.github.io/retrofit/: Converters By default, Retrofit can only deserialize HTTP bodies into OkHttp's ResponseBody type and it can only accept its RequestBody type for @Body.

Converters can be added to support other types. Six sibling modules adapt popular serialization libraries for your convenience.

Gson: com.squareup.retrofit2:converter-gson Jackson: com.squareup.retrofit2:converter-jackson Moshi: com.squareup.retrofit2:converter-moshi Protobuf: com.squareup.retrofit2:converter-protobuf Wire: com.squareup.retrofit2:converter-wire Simple XML: com.squareup.retrofit2:converter-simplexml JAXB: com.squareup.retrofit2:converter-jaxb Scalars (primitives, boxed, and String): com.squareup.retrofit2:converter-scalars Which does not implies json.org, if I understand corrently. With your permission I would change the JSON-Handeling to Jackson.

I actually wrote this code years ago, before I enrolled into computer science and were able to write good code. At the moment I actually don't have time for active maintainance, but I really appreciate code contribution and will review and merge. Also looking for a co maintainer if you'd like to stick around. Would love to improve the App. I myself am also just a beginner, so we will see how much I can do :)

When working on your patch also note this repo I started when dissecting the nextbike api: https://github.com/h0chi/nextbike-api-reverse-engineering I saw that already. Great work! There is also a offical API documentation from nextbike now. https://api.nextbike.net/api/documentation I was wondering where did you get the apikey for the application? Did you reverse-engineer it? They say, that they give out API-Keys upons request...

Something completely different: Is the app actually working for you? Few hours after I received your issue, I (unrelated) tried to rent a bike, didn't work and later that day nextbike sent me a mail that my "account has been locked due to payment method bla bla". I works for me. But I also had this Issue... We need to work on the Errorhandeling so that the users knows why the app does not work!

cybre-finn commented 4 years ago

Which does not implies json.org, if I understand corrently. With your permission I would change the JSON-Handeling to Jackson.

Just give it a try. If I totally dislike something we'll discuss this during review. I really appreciate any contribution. At the moment I just don't have the time to get deeper into this.

I saw that already. Great work! There is also a offical API documentation from nextbike now. https://api.nextbike.net/api/documentation I was wondering where did you get the apikey for the application? Did you reverse-engineer it? They say, that they give out API-Keys upons request...

I reversed engineered (see here: https://github.com/h0chi/nextbike-api-reverse-engineering/issues/1). The official documentation had some flaws or wasn't up to date or something... and at the time not even available.

I works for me. But I also had this Issue... We need to work on the Errorhandeling so that the users knows why the app does not work!

I have time for some future planning for this app next month. If you'd like to co maintain we can have a jitsi call or whatever and talk a little bit about the roadmap.

lgehr commented 4 years ago

Just a quick update, that I am still working on it but it takes more time than I thought. Will open a WIP Pull-request soon.

lgehr commented 4 years ago

@h0chi Login is implemented with Retrofit & Jackson. I will wait for your feedback until I start with changing the RentActivity too.

lgehr commented 3 years ago

I have time for some future planning for this app next month. If you'd like to co maintain we can have a jitsi call or whatever and talk a little bit about the roadmap.

Do you maybe have time for a jitis call the upcomming weekends. For roadmap and finally finishing the huge PR? :)