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

Overload resolution ambiguity with adapter(RenderableArrayAdapter) #14

Closed
corbt opened this issue May 20, 2015 · 9 comments
Closed

Overload resolution ambiguity with adapter(RenderableArrayAdapter) #14

corbt opened this issue May 20, 2015 · 9 comments

Comments

@corbt
Copy link
Contributor

corbt commented May 20, 2015

I have the following test activity in kotlin that is causing an error that appears to be related to the implementation of RenderableArrayAdapter:

import android.app.Activity
import android.os.Bundle

import android.content.Context;
import android.util.Log
import android.widget.Button;
import android.widget.LinearLayout;
import android.widget.ListView
import android.widget.TextView;
import trikita.anvil.RenderableView;

import trikita.anvil.RenderableView;
import trikita.anvil.BaseAttrs.*;
import trikita.anvil.Nodes
import trikita.anvil.RenderableArrayAdapter
import trikita.anvil.v10.Attrs.*;

class UsersList: Activity() {
    override fun onCreate(b: Bundle?) {
        super<Activity>.onCreate(b)

        var testNames: Array<String> = array("Bob Jones", "John Black", "Other Name")

        setContentView(object: RenderableView(this) {
            public override fun view() =
                    v<LinearLayout> {
                        - size(FILL, FILL)
                        - orientation(LinearLayout.VERTICAL)

                        v<TextView> {
                            - text("Names")
                        }

                        v<ListView> {
                            adapter(object : RenderableArrayAdapter<String>(testNames) {
                                override fun itemView(p0: Int, p1: String?): Nodes.ViewNode? {
                                    throw UnsupportedOperationException()
                                }
                            })
                        }
                    }
        });
    }
}

The compiler throws the following error for the adapter() function call:

Error:(37, 29) Overload resolution ambiguity:
public open fun adapter(arg: android.widget.ListAdapter!): trikita.anvil.Nodes.AttrNode! defined in trikita.anvil.v10.Attrs
public open fun adapter(arg: android.widget.Adapter!): trikita.anvil.Nodes.AttrNode! defined in trikita.anvil.v10.Attrs
public open fun adapter(arg: android.widget.SpinnerAdapter!): trikita.anvil.Nodes.AttrNode! defined in trikita.anvil.v10.Attrs

It looks like this is because RenderableArrayAdapter implements ListAdapter and SpinnerAdapter (and I guess Adapter as well). But since there are different implementation bodies the method doesn't resolve. Is there something I'm doing wrong here, or should adapter be combined into a single method call that dispatches internally based on its argument type?

@zserge
Copy link
Collaborator

zserge commented May 20, 2015

@corbt Thanks for trying Anvil and for reporting this. There is nothing wrong with your code, and your assumption about the adapters type hierarchy is absolutely correct.

Since Attrs.java code is generated from android.jar it doesn't notice that ListAdapter, SpinnerAdapter and just Adapter are inherited, so it generates 3 separate functions for adapter() binding.

In java I currently cast the class like adapter((ListAdapter) myRenderableAdapter), but I admit that it's not a solution. A possible solution would be to ban all adapter() calls in generator, and rewrite it manually as a single call, checking for the view instance and casting/dispatching it there. I'm not sure if banning ListAdapter and SpinnerAdapter but leaving just Adapter would help.

@corbt
Copy link
Contributor Author

corbt commented May 20, 2015

Ok, makes sense. That workaround will work fine for now. For anyone using Kotlin, here's how I cast the adapter to make that work:

var testAdapter = object: RenderableArrayAdapter<String>(testNames) {
    override fun itemView(i: Int, t: String?): Nodes.ViewNode? {
        println("rendered a list item")
        return v<TextView> { - text(t) }
    }
}
...

v<ListView> {
    - adapter(testAdapter: ListAdapter)
}

@zserge
Copy link
Collaborator

zserge commented May 20, 2015

I have to warn you about the main caveat of Anvil. Inside your view() functions you should never do heavy operations. All the operations inside the view() are executed on each Anvil.render() cycle.

This means that if you keep a reference to your adapter in your activity - it will be created once, and on each render cycle no extra actions will happen, because the same adapter is reused in each rendering cycle. However, if you call it like adapter(new RenderableArrayAdapter()...) - then a new instance of adapter is created each time, I don't know how equals() is implemented in adapters, but if they won't be equal - then setAdapter() will be called on each rendering cycle causing re-rendering of your listview.

Rendering cycles do not happen too often, they happen every time the on...Listener callback is called with any of your views (e.g. on every user interaction). But it can slow down your app significantly if you handle scrolls, drags, touches or other "frequent" events.

On the other hand, I personally find it appropriate to write - onClick(v -> { doSomething() }), even though a new OnClickListener will be created on each call. The reason is that OnClickListener is a very lightweight object, no heavier than a couple of Integers, or some small List, so it should not affect the performance.

@zserge
Copy link
Collaborator

zserge commented May 20, 2015

Now back to the original question, I think we can safely remove adapter(ListAdapter) and adapter(SpinnerAdapter), leaving only adapter(Adapter) instead. setAdapter is an abstract method. For ListViews and SpinnerViews (they are children of the abstract AdapterView class) it will call the actual implementations.

I'm still looking at ExpandableList, which uses two setAdapter() methods, and the one with ListAdapter throws a runtime exception. That's a bummer.

@zserge
Copy link
Collaborator

zserge commented May 20, 2015

@corbt You may fetch Anvil from tip and try it with your sources. I've just pushed the fixed generated Attrs.java, so now no type cast it needed. It should work for all AdapterViews (List, Spinner, Grid), and for ExpandableAdapterView as well.

@corbt
Copy link
Contributor Author

corbt commented May 26, 2015

After an extended weekend I'm back at it.

It looks like that change fixed the issue, at least for my very simple test case. The only thing to note is that I had to switch my import from import trikita.anvil.v15.Attrs.*; to import trikita.anvil.v10.Attrs.*; for the disambiguation to take effect. I'm not sure what the difference is between v10 and v15, but v10 seems to be working at any rate.

@zserge
Copy link
Collaborator

zserge commented May 26, 2015

v10 is for API level >= 10 (Android 2.3.3, 99% of the devices)
v15 is for API level >= 15 (Android 4.x, 96% of the devices)
I've just pushed a fix to v15, there are some new classes/methods that I forgot to blacklist during the code generation.

@corbt
Copy link
Contributor Author

corbt commented May 26, 2015

Makes sense. With the new code push v15 is working again, so I've switched back to that because it's lower than the API level I'm supporting anyway.

@zserge
Copy link
Collaborator

zserge commented Jun 14, 2015

This has been fixed for both v10 and v15, so I'm closing the issue

@zserge zserge closed this as completed Jun 14, 2015
sgrekov added a commit to sgrekov/anvil that referenced this issue Sep 28, 2019
* add sample for constraint dsl

* remove unnecessary DSL prefixes
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