CODEWITHSUNDEEP
androidkotlinandroid-viewandroid-custom-view
isuPatches
isuPatches

Reputation: 7484

Android: Abstract custom view and common layout inflation

So, I've done some research and it seems that view inflation within the init/constructor of an abstract base class is not really a best practice. I understand that's because the base class initializer happens before the init/constructor of the derived class. Since the abstract class is non-final there's a nice IDE message about this being leaked in the init block.

Here is what I'm after though:

abstract class Foo @JvmOverloads constructor(
    context: Context,
    attrs: AttributeSet? = null,
    defStyleAttr: Int = 0
) : FrameLayout(context, attrs, defStyleAttr) {

    private val myView: View

    init {
        // todo@patches fix leaking "this"
        View.inflate(context, R.layout.view_foo, this)
        myView = requireNotNull(findViewById(R.id.my_view))
    }
}

class Bar @JvmOverloads constructor(
    context: Context,
    attrs: AttributeSet? = null,
    defStyleAttr: Int = 0
) : Foo(context, attrs, defStyleAttr)

I really don't want to have to add anything to the init of the derived class or make myView a nullable/mutable variable that is set later in the abstract class.

Does anyone else find this slightly frustrating or have any advice? It seems like it would be not uncommon to want to inflate the same layout from a base class.

Upvotes: 2

Views: 1269

Answers (2)

fHate
fHate

Reputation: 194

Too late, but maybe someone found this useful.

You can simply initialize your View or Binding after constructor call. Just use lazy initialization, or initialize inside onFinishInflate method.

Upvotes: 0

Tenfour04
Tenfour04

Reputation: 93511

Leaking this in a constructor is dangerous because the object you leak it to could start accessing its members before the constructor even finishes, so it might not be ready. You could get NPEs even for non-nullable Kotlin properties, or other odd behavior.

In the case of LayoutInflator.inflate, this appears to not be an issue, if only because Android's built-in views frequently pass this as the parent to the inflate() method. For example, DatePicker's constructor instantiates a DatePickerSpinnerDelegate, which passes that DatePicker instance to inflate(), all happening before DatePicker's constructor has returned.

When you pass a view as the parent to inflate(), by following the call chain I see two things happen to the parent. It calls getContext() on that parent, and it calls addView() on that parent if addToRoot is true. So I think leaking this is safe so long as you don't override addView() to do additional work that relies on members that you set up after your call to inflate(). ButaddView()also internally callsrequestLayout()andinvalidate()`, so the same concern applies to those.

In most cases your custom ViewGroup would be a subclass of an existing Android ViewGroup class, so you wouldn't need to override those methods.

Unfortunately, we can only infer this behavior by inspecting the code. It's not terribly reassuring that it's not guaranteed safe by the documentation, but as far as I know, we just have to accept that its probably safe. Maybe an issue should be opened on AOSP. This warning doesn't even appear if you write your same code in Java, but the risk is the same.

Suppressing a warning shouldn't mean that you are ignoring a warning or just hacking at your code. It means, "I acknowledge the failure mode and have checked that my code will not fail in this way." If that weren't the case, it would be a compiler error, rather than a warning. In Kotlin, the suppression annotation you can use is @Suppress("LeakingThis").

Upvotes: 6

Related Questions