zul-aji / CodeRef_LabPro

Repository for Code Refactoring Lab project 1
0 stars 0 forks source link

Conditional Complexity (and Unit Test) #2

Open zul-aji opened 1 year ago

zul-aji commented 1 year ago
Example 1 ## Code Snippet - DebtsAdapter.kt ```kotlin override fun onBindViewHolder(holder: DebtListViewHolder, position: Int) { if (debtList[position].date == "Due date not set") { holder.untilDate.text = holder.itemView.context.getString(R.string.date_unpicked) } else { holder.untilDate.text = holder.itemView.context.getString(R.string.until_date, debtList[position].date) } .... //if a debt is due today if (debtList[position].dateInteger == TodayDateInt) { if (fragmentId == "MyDebtsFragment" || fragmentId == "SomeoneOwesMeFragment") { .... } holder.untilDate.text = holder.itemView.context.getString(R.string.due_today, "Today!") //if a debt date is picked and the due is past } else if (debtList[position].dateInteger < TodayDateInt) { if (fragmentId == "MyDebtsFragment" || fragmentId == "SomeoneOwesMeFragment") { .... } val datePassed = getDatePassed(debtList[position].date) holder.untilDate.text = holder.itemView.context.getString(R.string.past_due, datePassed.toString()) } holder.itemView.setOnClickListener { val nextPage = Intent(holder.itemView.context, AddDebtDetails::class.java).apply { .... } holder.itemView.context.startActivity(nextPage) } } ``` ## Code Description The code snippet above shows a complex cluster of if else which is also enhanced with long lines of codes. The screen was made to show different holder colors for different conditions and screen which makes it hard to understand for a single function. ## Flaw Reason For future development, it will be hard to add modifications as this violates the OCP (Open/Close Principle). The nested and different conditional will make it hard to track and will also make it harder to change in the future
Example 2 ## Code Snippet - AddDebtDetails.kt ```kotlin binding.saveBtn.setOnClickListener { .... isAllFieldsChecked = checkAllFields() debtDate = fillDate(debtDate) if (isAllFieldsChecked) { when (intent.getStringExtra(PassedData.ADD_SAVE)) { "add" -> { .... } "save" -> { .... } finish() } } ``` ## Code Description The code snippet above is actually not very complicated but the length of the code can make it quite confusing for further development (the code snippet above is summarized, it is longer in the real class). The when condition `intent.getStringExtra(PassedData.ADD_SAVE` is not very informative which also could mislead in further development ## Flaw Reason The conditional itself is not very hard to complex but it could be hard to understand for other people encountering and could lead to increased development time.
LidiaIvanova commented 1 year ago

1

zul-aji commented 1 year ago

Unit Test

Code Flaw

The unit test I took for this Conditional Complexity is the second example. The second example shows what happens when we click on the save button and the nested if and when is not very informative. This code flaw is also made more complicated by the length of the code.

Unit Test Description

class ConditionalComplexity(private val callFunction: DebtFunCall) {

    fun isDebtSavedOrUpdated(
        allFieldChecked: Boolean,
        buttonState: String,
        buttonCommand: String
    ): Boolean {
        if (!allFieldChecked) {
            return false
        }
        if (buttonState.isEmpty()) {
            return false
        }
        return if (buttonCommand == "Add") {
            callFunction.addData()
            true
        } else if (buttonCommand == "Update") {
            callFunction.updateDebt()
            true
        } else {
            false
        }
    }

    interface DebtFunCall {
        fun addData()
        fun updateDebt()
    }
}

The test is centered around the save button, which adds or edits an existing debt. The buttonState variable is for informing the button whether it is in an add or edit page and it can't be both or none of them. Both adding and editing need to make sure that all the fields are checked and then check on whether the command for the button is Add or Update. If the command is Update, then the debtId needs to be passed so it will update the correct debt. If all the requirements are met, both the Add and Update commands will then call on their respective function. As there are dependencies in the tested class, the test cases would need a mock dependency.

@Mock
private lateinit var mockDependency: ConditionalComplexity.DebtFunCall

Test 1

@Test
fun checkFields_notAllFilled_falseReturn() {

    val conditionalComplexity = ConditionalComplexity(mockDependency)

    val result = conditionalComplexity.isDebtSavedOrUpdated(
        false,
        "",
        ""
    )
    assertThat(result).isFalse()
}

The test asserts false when not all debt details fields are filled.

Test 2

@Test
fun checkFields_allFilledNoState_falseReturn() {

    val conditionalComplexity = ConditionalComplexity(mockDependency)

    val result = conditionalComplexity.isDebtSavedOrUpdated(
        true,
        "",
        ""
    )
    assertThat(result).isFalse()
    verify(mockDependency, times(0)).addData()
    verify(mockDependency, times(0)).updateDebt()
}

The test asserts false when all the fields are filled but the buttonState is empty which is an error.

Test 3

@Test
fun callFunction_addCommandCallUpdateFunction_falseOutput() {

    val conditionalComplexity = ConditionalComplexity(mockDependency)

    val result = conditionalComplexity.isDebtSavedOrUpdated(
        true,
        "Add",
        "Update"
    )
    assertThat(result).isTrue()
    verify(mockDependency, times(0)).addData()
    verify(mockDependency, times(1)).updateDebt()
}

The test asserts true when the fields, state, and command are not empty, but the state and command are not the same resulting in an output that is not expected.

Test 4

@Test
fun callFunction_addCommandFunctional_trueReturn() {

    val conditionalComplexity = ConditionalComplexity(mockDependency)

    val result = conditionalComplexity.isDebtSavedOrUpdated(
        true,
        "Add",
        "Add"
    )
    assertThat(result).isTrue()
    verify(mockDependency, times(1)).addData()
    verify(mockDependency, times(0)).updateDebt()
}

The test asserts true when the state and command are to Add and resulting in an output that is expected.