2
\$\begingroup\$

I have a Singleton manager class for managing request model and its attributes.

My main aim is update object observers when some of the object attributes changes.

class FlightSearchManager {

    private val _flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData()
    val flightSearchRequestModelLiveData : LiveData<FlightSearchRequestModel> get() = _flightSearchRequestModelLiveData

    var flightSearchRequestModel : FlightSearchRequestModel

    init {
        flightSearchRequestModel =
            FlightSearchRequestModel(
                null,
                null,
                null,
                null,
                null,
                null,
                null,
                null,
                null)
    }

    fun updateFlightSearchRequestModel(_arriveAirport : String? = null,
                                       _arriveAirportId: Int? = null,
                                       _departureAirport : String? = null,
                                       _departureAirportId: Int? = null,
                                       _departureDate : String? = null,
                                       _isRoundTrip : Boolean? = false,
                                       _paxTypeCount : List<PaxTypeCount>? = null,
                                       _returnDate : String? = null,
                                       _seatClass : String? = null) {
        flightSearchRequestModel.apply {
            arriveAirport = _arriveAirport ?: arriveAirport
            arriveAirportId = _arriveAirportId ?: arriveAirportId
            departureAirport = _departureAirport ?: departureAirport
            departureAirportId = _departureAirportId ?: departureAirportId
            departureDate = _departureDate ?: departureDate
            isRoundTrip = _isRoundTrip ?: isRoundTrip
            paxTypeCount = _paxTypeCount ?: paxTypeCount
            returnDate = _returnDate ?: returnDate
            seatClass = _seatClass ?: seatClass
            // Notify all listeners of live data
            _flightSearchRequestModelLiveData.value = this
        }
    }
}

When I want to notify observers I call:

flightSearchManager.updateFlightSearchRequestModel(_seatClass = "1")

This solution does not seem good solution. How can I write it more intelligently?

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

To give you valuable feedback on your usecase, I would need more information, like: How do listeners listen? How does the caller look like?

How exactly does _flightSearchRequestModelLiveData.value = this notify all listeners of live data? Its not a function call, so how is an event created and pushed?!

Instead I will give some pattern advices you may need.

Use constructor initialisation

  1. Kotlin enables you to set fields over constructors. Even if your class is a singleton and all values are default - creating a constructor will enable to test your class easily (and other benefits).

So instead of:

class FlightSearchManager {

    private val _flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData()
    val flightSearchRequestModelLiveData : LiveData<FlightSearchRequestModel> get() = _flightSearchRequestModelLiveData

    var flightSearchRequestModel : FlightSearchRequestModel

try to go for:

class FlightSearchManager(
    val flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData(),
    var flightSearchRequestModel : FlightSearchRequestModel = FlightSearchRequestModel()
) {
  1. If you already have a constructor for a class and it is valid to have all fields to have default values - don't force the caller to set them!

This code is an anti-pattern:

flightSearchRequestModel =
            FlightSearchRequestModel(
                null,
                null,
                null,
                null,
                null,
                null,
                null,
                null,
                null)

What you should go for would be:

data class FlightSearchRequestModel(
   val propertyOne : String? = null, 
   val propertyTwo: Int? = null,
   ...
)

FlightSearchManager {
  val model = FlightSearchRequestModel()
}

Parameters

It is seen as code smell, when your method has too many parameters, so I would recommend to reduce the number of those in fun updateFlightSearchRequestModel.

You could create a single function for every parameter change. Of course it means more lines to write, but is cleaner, straight forward and not as error-prone to side effects since you don't change ~20 things in one method.

Naming convetions

I would guess that you coded with something like C# before, because of the usage of underscores _ in variable names. Please try to follow the appropreate convetion for this language (Kotlin), as well as in Java:

Local variables, instance variables, and class variables are also written in lowerCamelCase. Variable names should not start with underscore (_) or dollar sign ($) characters, even though both are allowed. This is in contrast to other coding conventions that state that underscores should be used to prefix all instance variables.

Constants:

Constants should be written in uppercase characters separated by underscores. Constant names may also contain digits if appropriate, but not as the first character.

Source

Fields like this:

private val _flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData()

would become:

private val flightSearchRequestModelLiveData : MutableLiveData<FlightSearchRequestModel> = MutableLiveData()

Functions like this

fun updateFlightSearchRequestModel(_arriveAirport : String? = null,
                                   _arriveAirportId: Int? = null,

would become:

fun updateFlightSearchRequestModel(arriveAirport : String? = null,
                                   arriveAirportId: Int? = null,

Immutability

Immutable code is easier to debug, to understand and to test. Favor val over var in (almost) every case.

If you really need to change something by reference, at least create a completely new immutable object. Example:

This

flightSearchRequestModel.apply {
            arriveAirport = _arriveAirport ?: arriveAirport
            arriveAirportId = _arriveAirportId ?: arriveAirportId
            ...
}

could become this:

flightSearchRequestModel = copy(arriveAirport = arriveAirport, arriveAirportId  = arriveAirportId, ...)
\$\endgroup\$

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.