How can I improve three methods which slightly do different work

Question

I have the following methods which return specific values of an object depending on where they are called from.

 fun getRoamingStatusErrorItem(): SettingItem {
        return SettingRoamingItem(
            isRoaming = false,
            processingText = "",
            isEnabled = false,
            isErrorVisible = true,
            isProgressVisible = false)
    }

    fun getRoamingStatusProgressItem(): SettingItem {
        return SettingRoamingItem(
            isRoaming = false,
            processingText = "",
            isEnabled = false,
            isErrorVisible = false,
            isProgressVisible = true)
    }

    fun getRoamingStatusProcessingItem(text: String): SettingItem {
        return SettingRoamingItem(
            isRoaming = false,
            processingText = text,
            isEnabled = false,
            isErrorVisible = false,
            isProgressVisible = false)
    }

As you can see these methods just represent different states of an object type; I a want to understand if this is a good coding practise to use it this way or can I refactor it in a better fashion

This is not Java, you can remove the tag – Bentaye Jun 21 '19 at 12:59 — Bentaye, Jun 21 '19 at 12:59

Roland · Accepted Answer · 2019-06-21T13:18:25.160

2

If SettingRoamingItem is under your control, you can just give it some default values, e.g.:

class SettingRoamingItem(
  val isRoaming : Boolean = false,
  val processingText : String = "",
  val isEnabled : Boolean = false,
  val isErrorVisible : Boolean = false,
  val isProgressVisible : Boolean = false)

then your function calls can be simplified to:

fun getRoamingStatusErrorItem() = SettingRoamingItem(isErrorVisible = true)
fun getRoamingStatusProgressItem() = SettingRoamingItem(isProgressVisible = true)
fun getRoamingStatusProcessingItem(text : String) = SettingRoamingItem(processingText = text)

But then: why would you even want such functions?

edited Jun 21 '19 at 13:18

answered Jun 21 '19 at 13:10

Roland

22,259
4
57
84

But then: why would you even want such functions? - to track object states depending on API calls and bind these to UI – User3 Jun 21 '19 at 13:20
I mean like: if you can write `SettingRoamingItem(isErrorVisible = true)` vs `getRoamingStatusErrorItem()`... but that's probably just me... – Roland Jun 21 '19 at 13:22

score 1 · Answer 2 · answered Jun 21 '19 at 13:03

There isn't too much that you can do without changing the whole class. The only "simple" thing would be to add another constructor that doesn't have the isRoaming parameter: in all three examples, that value is false. When you always pass the same value: don't pass the value, but use constructor telescoping to provide that value as "default".

Beyond that, if you are willing to invest more time, you should look into use the builder pattern here. Like:

SettingRoamingItemBuilder().withText("").withProgressVisible(true)..
  .build()

The point would be to identify those values for all these arguments that are most common, and to use those as defaults, so that only those places that deviate from the default have to make a call to change the default setting accordingly.

score 1 · Answer 3 · answered Jun 21 '19 at 13:05

You could use a Builder pattern for SettingRoamingItem and have isRoaming = false, processingText = "", isEnabled = false, isErrorVisible = true, isProgressVisible = false for default values. Then use something like .withProcessingText("text").enableVisibleError().build() to get the instance you need for your getters.

Xavier Bouclet · Answer 4 · 2019-06-21T14:10:05.097

You could rewrite your constructor with default values :

class SettingRoamingItem(
        var isRoaming: Boolean = false,
        var  processingText :String = "",
        var isEnabled: Boolean = false,
        var isErrorVisible: Boolean = true,
        var isProgressVisible: Boolean = false)



fun getRoamingStatusErrorItem(): SettingItem = SettingRoamingItem(isErrorVisible = true, isProgressVisible = false)


fun getRoamingStatusProgressItem(): SettingItem = SettingRoamingItem(isErrorVisible = false, isProgressVisible = true)


fun getRoamingStatusProcessingItem(text: String): SettingItem = SettingRoamingItem(processingText = text,
        isErrorVisible = false,
        isProgressVisible = false)

How can I improve three methods which slightly do different work

4 Answers4