aashishksahu / SafeSpace

A safe place for your valuable information
GNU General Public License v3.0
144 stars 9 forks source link

Add option to share files stored in vault with other apps #50

Closed starry-shivam closed 3 months ago

starry-shivam commented 3 months ago

While on that, also upgrade AGP version. Closes #47

aashishksahu commented 3 months ago

@starry-shivam This looks good overall, you really know android 💯. Just take care of a few little things for which I've added comments :)

starry-shivam commented 3 months ago

@aashishksahu Sorry but I can't find the comments which you've added, perhaps you forgot to submit the review ?

aashishksahu commented 3 months ago

Thanks for the corrections @starry-shivam. I'll merger these changes after completing mine.

aashishksahu commented 3 months ago

@starry-shivam Besides this, can you please give me some suggestions to improve the quality of the code base, specifically the main activity, it's way too long and also the recycler views, they look like a mess to me. I feel like it's time I focus more on stability rather than features. After the next release, I'll start working on some refactoring.

starry-shivam commented 3 months ago

@starry-shivam Besides this, can you please give me some suggestions to improve the quality of the code base, specifically the main activity, it's way too long and also the recycler views, they look like a mess to me. I feel like it's time I focus more on stability rather than features. After the next release, I'll start working on some refactoring.

To be honest, I think there are architectural issues with the app, for example, you should avoid placing any logic/backend related code in activities and fragments because they get reloaded in many cases, such as orientation changes, system theme changes and more, which could lead to unexpected behaviors and unnecessary recomputation of certain functions when they're not really needed.

Instead, you should move any data/backend-related logic to something called ViewModels (Take a look at the MVVM architecture). ViewModels are designed to survive orientation and similar changes, which typically reload activities and fragments. According to MVVM's guidelines, any data/logic/backend-related tasks should reside in the ViewModels, while your UI layer (i.e., Fragments) should only observe and react to changes in data emitted by ViewModels. This not only makes your app resistant to orientation changes, improving the overall UX, but also improves organisation and maintainability of the code, since you don't have to mess with the UI layer when fixing or improving backend related functions, and vice versa.

Additionally, I strongly suggest using Constants (Enums, sealed classes, etc., or even just a dedicated constant class) instead of relying on literal integer values to denote the status of certain operations. Over time, this approach can lead to unexpected bugs and maintainability issues, like when you inadvertently or deliberately change the return status of certain operations and codebase has became large enough.

If you ask me I also recommend considering a switch to Jetpack Compose at this point, as it basically the direction modern Android development has moved towards. XML and the view system are now considered outdated, and transitioning to Compose offers numerous benefits, such as easy implementation of reactive UI & custom layouts, improved support for features like Material You, improved reusability of UI components, and significantly reduced boilerplate code. For example, implementing a scrolling view with recycler views requires adding three different classes and two XML views, whereas Jetpack Compose allows you to achieve similar functionality in just 20 to 50 lines of code.

These are some immediate suggestions I've come up with after reviewing the code. While there are ofcourse more ways to improve it, I think you should start with the ones I've mentioned before proceeding further.

aashishksahu commented 3 months ago

@starry-shivam Thank you very much for your suggestions, I’ll start the code renaissance soon, I hope you can guide me once in a while :)

starry-shivam commented 3 months ago

@starry-shivam Thank you very much for your suggestions, I’ll start the code renaissance soon, I hope you can guide me once in a while :)

Sure! Just give me a ping :)