asamm/locus-api

make locus.api.objects.extra.Location non final

bekuno opened this issue · 5 comments

bekuno commented

In WhereYouGo the implementation of class menion.android.whereyougo.geo.location.Location was realized with
public class Location extends locus.api.objects.extra.Location {.
(see https://github.com/cgeo/WhereYouGo/blob/master/src/main/java/menion/android/whereyougo/geo/location/Location.java)
(change to inherit in this way was in cgeo/WhereYouGo@aece42d)

To make a switch easy to implementation 'com.asamm:locus-api-android:0.9.21'(from 0.2.7) it would be fine if Location is non final.

menion commented

Hello Bernd,
may you please explain to me two questions ...

  1. why you extend Location from Locus API and use it as the main object for storing geographical coordinates?
  2. is there any reason to use Locus API at all?

Location in Locus API is quite a complex class that keeps data also for various external BT/ANT sensors. Also as I see, your extension use only one additional constructor from the basic Android Location object, nothing more.
So isn't better, to use your own version or, keep a copy (as was in WhereYouGo probably before), optimized exactly for the WhereYouGo app?

It is not a problem to use Locus API objects, just a little weird :).

bekuno commented

Currently I try to upgrade WhereYouGo to the recent locus-api version.
There is no change necessary/planned in the function of WhereYouGo with Locus.

The design decision to inherit from locus.api.objects.extra.Location instead using an own class was taken by biylda in commit cgeo/WhereYouGo@aece42d on 7 Apr 2016.
The situation now is that Location as a kotlin class is final by default, but before as java class non final.

By the way, this is the only class in which this has been realised.

menion commented

Thanks for the explanation.
I saw the mentioned commit and I'm not sure, why this change was made. Objects in Locus API (at least the old version) were identical to object in WhereYouGo, when I made it open source. The reason was to detach dependency on Locus API and also some private tools I use in Locus Map/GIS projects.

Making classes that are based on Storables open, is a little bit dangerous. The Location object from Locus API is not perfectly prepared for extensions.

So if there will be voting, I definitely vote for creating an own version of the Location object in WhereYouGo. To be true, I see no problem in copy content from Locus API directly into WhereYouGo. With this, you will be safe and resistant against possible future updates in Locus API.

Anyway if you do not want to invest more time into this, let me know and I'll confirm your PR ... on your own risk :).

bekuno commented

Currently I have not really time to create a own class inside WhereYouGo.
(Additionally, I don´t know if it make sense to use a Kotlin class inside Java or to migrate the kotlin version back to java.)

So for now it would be fine to confirm the PR.

I hope that there will be another solution at some point.
For this I will create a WhereYouGo issue.

menion commented

Oki, understand.
Version [0.9.22] just published with Location class made open.

Offtopic: mixing Java & Kotlin is no problem and most projects use it. I do not in Locus Map as well > new code Kotlin, old remains in Java code.