1
\$\begingroup\$

I had an idea recently of simplifying some of the code I already have in my Android app that uses Kotlin. I somewhat new to Kotlin, so any opinion would be very appreciated.

Before giving you what idea I had, I will give a base example of what code I have right now:

Lets say that in my app I have a functionality for user to log in and save his credentials for automatic future logging in. Here it is in code.

object MyRepository {
 // Plenty of irrelevant code here...
 fun getUserInfo(): User? {
  // Here we fetch the local data storage and check if any credentials are saved
  return userInfoDao.getUserInformation()
 }

 fun setUserInfo(user: User) {
  // Here we save user credentials to local data storage for future uses
  userInfoDao.saveUserInformation(user)
 }
}

This lead me to think that something like this could be done in a simpler manner, and here is the example:

object MyRepository {
 var userInfo: User?
   get() = userInfoDao.getUserInformation()
   set(value) = userInfoDao.saveUserInformation(value)
 }
}

This of course is much simpler version of what was written above. However, what I am curious is how good of a practice this is in terms of Kotlin standards and general code readability?

\$\endgroup\$
4
  • \$\begingroup\$ "This of course is much simpler version of what was written above." - How much have you omitted really? Please note that in order to give the best review possible, we need to get as much context as possible. \$\endgroup\$ Nov 17, 2020 at 17:47
  • \$\begingroup\$ @SimonForsberg what I am talking about is that in my opinion, using one variable instead of 2 functions is a much simpler approach to coding. What I omitted has no contextual benefits to the code I provided. \$\endgroup\$
    – vt-dev0
    Nov 17, 2020 at 17:49
  • \$\begingroup\$ I will personally let it be for now, but I would like to take the opportunity to point out - in case you are not aware - that there is a close reason on this site which is: Missing Review Context: Code Review requires concrete code from a project, with enough code and / or context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site. \$\endgroup\$ Nov 17, 2020 at 17:52
  • \$\begingroup\$ I would personally give this a green signal. I can't say much about it as am myself very new to the development world, but our team has a 2yo product with a significant database and we use code similar to this for accessing sharedPreferences but not really in local databases \$\endgroup\$ Nov 17, 2020 at 18:56

1 Answer 1

5
\$\begingroup\$

Generally speaking, I would not expect a database call to be made simply by referring to a property, such as with val user = MyRepository.userInfo. If a database call is being made, it's better to have a function for it.

I'd even argue that it would even be better to name the function fetchUserInformation than getUserInformation to be clear that this is not just a simple getter.

\$\endgroup\$
1
  • 2
    \$\begingroup\$ To elaborate on this, I would use getters/setters on properties exclusively for computed properties, i.e. derived from some other properties in the class. \$\endgroup\$
    – Rubydesic
    Jan 7, 2021 at 14:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.