Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception is thrown with empty message when trp_hourFormat attribute has invalid value. #8

Closed
pelmenstar1 opened this issue Jun 19, 2021 · 2 comments

Comments

@pelmenstar1
Copy link
Contributor

pelmenstar1 commented Jun 19, 2021

If invalid value is passed to enum property, we will get a build error:
AAPT: error: '420' is incompatible with attribute trp_hourFormat (attr) enum [FORMAT_SYSTEM=0, FORMAT_12=1, FORMAT_24=2]

Code:

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    tools:context=".MainActivity">

    <nl.joery.timerangepicker.TimeRangePicker
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        app:trp_hourFormat="420" />

</LinearLayout>

But, nevertheless, if we pass a reference to integer with invalid value, we won't get any build error and code compiles successfully:
ints.xml

<resources>
    <integer name="some_invalid_value">3</integer>
</resources>

activity_main.xml

<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    tools:context=".MainActivity">

    <nl.joery.timerangepicker.TimeRangePicker
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        app:trp_hourFormat="@integer/some_invalid_value" />

</LinearLayout>

Full code

In runtime, we will get exception with no message:

     Caused by: java.lang.IllegalArgumentException
        at nl.joery.timerangepicker.TimeRangePicker$HourFormat$Companion.fromId(TimeRangePicker.kt:1160)
        at nl.joery.timerangepicker.TimeRangePicker.initAttributes(TimeRangePicker.kt:147)
        at nl.joery.timerangepicker.TimeRangePicker.<init>(TimeRangePicker.kt:114)
        at nl.joery.timerangepicker.TimeRangePicker.<init>(TimeRangePicker.kt:39)
        at nl.joery.timerangepicker.TimeRangePicker.<init>(TimeRangePicker.kt)

Library tries to handle such cases but unsuccessfully:

hourFormat = HourFormat.fromId(
attr.getInt(
R.styleable.TimeRangePicker_trp_hourFormat,
_hourFormat.id
)
) ?: _hourFormat // Sets public property to determine 24 / 12 format automatically

fun fromId(id: Int): HourFormat? {
for (f in values()) {
if (f.id == id) return f
}
throw IllegalArgumentException()
}

Method fromId never returns null, but throws exception with empty message on invalid value, but initAttributes expects that fromId returns null and assignes default value (HourFormat.FORMAT_12) if returned enum value is null.

So, what's expected behavior? To leave default value, or to throw exception. If the last one, it's better to throw exception with some message like 'Invalid attribute value(trp_hourFormat=value)'

@pelmenstar1
Copy link
Contributor Author

The same problem with trp_clockFace

_clockFace = ClockFace.fromId(
attr.getInt(
R.styleable.TimeRangePicker_trp_clockFace,
_clockFace.id
)
) ?: _clockFace

fun fromId(id: Int): ClockFace? {
for (f in values()) {
if (f.id == id) return f
}
throw IllegalArgumentException()
}

@Droppers
Copy link
Owner

I think it should throw an exception with a clear message, returning null and using the default value can be unexpected behavior for the user.

I currently don't have time (this week) to make any changes.

Droppers added a commit that referenced this issue Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants