Apps-android-commons: Refactoring the FileProcessor and GPSExtractor classes

Created on 13 Feb 2020  Â·  8Comments  Â·  Source: commons-app/apps-android-commons

Both of these classes contain old code that doesn't match how we currently use them. For example, the logic of extracting GPS data from a similar image and using them for that class is spread out between UploadMediaDetailFragment(some of the business logic seems to be totally missing) and FileProcessor. I restructuring those classes will lead to much more readable and maintainable code. What do you think?

assigned code-quality debated

Most helpful comment

Yup, I would be interested in working on that. I've already started working
on it, and I'll be refactoring some other old code that is related (ie. I
just created a License class instead of passing around these different keys
and text etc. everywhere.)

All 8 comments

I agree, those are very old classes, we might have missed them in the overhaul (oops!). Are you interested in working on these, @ilgazer ? I guess you are already familiar with the MVP architecture requirements (since I think you did some of the Retrofit refactor too)?

Yup, I would be interested in working on that. I've already started working
on it, and I'll be refactoring some other old code that is related (ie. I
just created a License class instead of passing around these different keys
and text etc. everywhere.)

Thank you! We appreciate it. :)

I will be taking over this ticket as it has had no activity for a month. Hope I am not stepping on your toes @ilgazer

Hi, I was just about to make a PR with some changes. I was really busy in
the past 2 weeks but I will have more time now. Is there any chance you
could take up another issue?

On Wed, 18 Mar 2020, 16:54 Seán Mac Gillicuddy, notifications@github.com
wrote:

I will be taking over this ticket as it has had no activity for a month.
Hope I am not stepping on your toes @ilgazer https://github.com/ilgazer

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/commons-app/apps-android-commons/issues/3408#issuecomment-600636718,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB2EXI5CUJ2GS4YBXC5ST23RIDHBDANCNFSM4KUZJSJQ
.

@ilgazer I am a full time resource on the project now and this ticket has been quite beneficial for me to get to grips with the upload process. I would prefer to keep going now that I have momentum but if you insist I will hand it back over, it being your ticket and all.

I understand and your reason to continue is quite reasonable. I would have
needed to rewrite a good portion of my work anyways as I mistakenly created
another class for storing GPS coordinates instead of reusing LatLng. Have
fun with the spaghetti code :))

>

I am waist deep in bolognese

Was this page helpful?
0 / 5 - 0 ratings