deadpixelsociety / passport

A Kotlin-based Android view validation library with a simple DSL.
MIT License
30 stars 1 forks source link

Out of order rules #2

Closed eddnav closed 6 years ago

eddnav commented 6 years ago

Not sure if this is expected, but I wanted to report it just in case:

When using:

Validator.validate(fragment, ValidationMethod.IMMEDIATE)

As weird as it sounds, it seems as if rules are evaluated out of order, for example, the following:

<android.support.design.widget.TextInputLayout
    android:id="@+id/fastingInputLayout"
    android:layout_width="0dp"
    android:layout_height="wrap_content"
    app:layout_constraintEnd_toEndOf="parent"
    app:layout_constraintTop_toTopOf="@id/bloodVolumeInputLayout"
    app:layout_constraintWidth_percent=".45">

    <android.support.design.widget.TextInputEditText
        android:id="@+id/fasting"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:hint="@string/label_pathological_fasting"
        android:inputType="numberDecimal"
        android:singleLine="true"
        tools:text="0.0" />

</android.support.design.widget.TextInputLayout>

With nothing on the field and:

...
rules<String>(fastingInputLayout) {
    notBlank(getString(R.string.validation_not_blank))
    rule({ it.toDoubleOrNull() != null }, { getString(R.string.validation_valid_decimal) })
    rule({ it.toDouble() >= 0 }, { getString(R.string.validation_equal_or_greater_than_0) })
}
....

Sometimes would go over the notBlank rule and fail on the second, a breakpoint indeed reveals the value of text to be "" so why would it pass that first rule? sometimes it would even pass the second one with an empty field as well, crashing the app on the last one.

In the end, what I did was this, which seems to work properly:

private fun validate(): Boolean {
    var pass = true

    if (!mValidator.validate(firstNameInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(lastNameInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(ageInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(weightInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(surgeryDescriptionInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(surgeryDurationInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(bloodVolumeInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(fastingInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(surgicalStressInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(hemoglobinInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(minHemoglobinInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(crystalloidsInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(colloidsInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(hemoderivativesInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(drugInfusionsInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(diuresisInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(aspirationInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(compressesInputLayout, ValidationMethod.IMMEDIATE)) pass = false
    if (!mValidator.validate(levinsTubeInputLayout, ValidationMethod.IMMEDIATE)) pass = false

    return pass
}

It has the added benefit of working like ValidationMethod.BATCH, but breaking on the first failed rule instead of concatenating all the failed messages, would be nice to have a way to do something like that with the activity, fragment based validate.

deadpixelsociety commented 6 years ago

Interesting. Thanks for the report! I'll take a look at it soon.

For your validation example, you'd want to see an option for the top level validate methods to stop a child view's validation when it hits the first failure and then continue on to the next child, correct? That sounds pretty doable.

deadpixelsociety commented 6 years ago

@eddnav Thanks again for the reports! I've pushed changes to now support a 'fail fast' mode that implements your use case. All views can be processed and rules processing will stop at the first failure.

I also found the issue with rules processing out of order and fixed that, as well as some QOL changes along the way.

Hope this helps!

eddnav commented 6 years ago

Works perfectly, thank you so much!