Add ViewActions for setting text of TextView about rxandroid HOT 13 CLOSED

You don't need a wildcard for Integer.
On Nov 24, 2014 7:07 AM, "Dale King" [email protected] wrote:

Should add these methods to ViewActions to subscribe a text view (which
also includes EditText and buttons) to a source of the text:

public static Action1<? super CharSequence> setText(TextView view)
public static Action1<? super Integer> setTextResource(TextView view)

—
Reply to this email directly or view it on GitHub
#80.

from rxandroid.

Probably true, but I just started with a cut and paste from existing methods in ViewActions that use ? super Boolean, which is probably not necessary either

from rxandroid.

Whenever I see something exotic like that it makes me pause and think "wait, why?"

We should avoid developer surprise. setTextResource should operate on straight-up integers.

from rxandroid.

Actually I think there is a reason for the ? super Boolean in the existing method and thus for the ? super Integer in my cut and paste. I am not an expert on this but this may help: http://stackoverflow.com/questions/4343202/difference-between-super-t-and-extends-t-in-java.

I think the idea is that it in addition to working with Boolean in the existing methods or Integer in my version it will also work with less strictly typed observables like Observable of Object or in my Integer case also Observable of Number. If that observable emitted an incompatible type you will get an error but could simplify things in a few cases.

Can we please move the discussion away from whether the wildcard is necessary onto actually getting the functions supported?

from rxandroid.

@dalewking Good point. We should definitely implement this. We can bikeshed the implementation details in the PR :)

from rxandroid.

The wildcard (with any bound) is not needed in either case here because Integer and Boolean cannot be subclassed nor you don't want any of its super classes (Number, Object). The lower bound of CharSequence on the other one makes sense because you want to allow subclasses like String, SpannableString, etc.

from rxandroid.

I'm sorry Jake, you are wrong. I cannot fault you since I didn't understand all the details of this until researching the topic for this discussion. I've gone round and round on this and my head is ready to explode. A lot of this comes from the confusing nature of "? super" vs "? extends".

If we were talking about ? extends Integer or ? extends Boolean then the fact that these are final would mean those were redundant expressions and the wildcard should definitely be eliminated.

But since the question here is about ? super Integer the fact that Integer is final is completely immaterial. It is certainly wrong to say that "? super Integer" never makes sense because Integer is final. It is wrong to say that we do not want any of the superclasses either, because this is not about what gets passed into the action, but on how the action would be implemented

It is also wrong to say that the bound on CharSequence makes sense because you want to allow subclasses, because it actually doesn't allow subclasses. ? super is not about subclasses so subclasses like String have no bearing here.

The case for super here is kind of difficult to see and really boils down to the implementation of the method.

Instead of the general case asked for here lets imagine we have an action like this that will print numbers:

Action1<? super Number> printNumberAction = new Action1<Number>()
    {
        @Override
        public void call(Number n) { System.out.println(n); }
    };

Imagine if we wanted to create an action on Integer that does the same thing. Rather than duplicating the code we would like to just return the existing action. So consider these 2 declarations:

Action1<Integer> foo1() { return printNumberAction; }
Action1<? super Integer> foo2() { return printNumberAction; }

In the case of foo1 that would be an error as Action1 is not compatible with Action1, but foo2 would compile and work just fine.

Applying it to this case it is easier to see for CharSequence. Imagine if we have a method like this:

public static Action1<Object> setTextFromObjectToString(final TextView view)
{
    return new ViewAction1<TextView, Object>(view)
    {
        @Override
        public void call(TextView view, Object o)
        {
            view.setText(o.toString());
        }
    };
}

Then I can implement the requested method simply as:

public static Action1<? super CharSequence> setText(TextView view)
{
    return setTextFromObjectToString(view);
}

One could actually make the case that we should really define this using the Object.toString version and not even have the CharSequence version.

from rxandroid.

To be honest, I understand it very well, I just never really read what you had (still haven't and full disclosure I didn't read your response). This project gets 0.01% of my attention so I rarely actually read these things fully.

One could actually make the case that we should really define this using the Object.toString version and not even have the CharSequence version.

No you can't. toString() != CharSequence and you'll throw away a lot of important information like spannables.

from rxandroid.

I'm implementing this so we can take the discussion to the PR

from rxandroid.

The question is not about my responses, it boils down to your statements, which I assume you did read:

You don't need a wildcard for Integer

The wildcard (with any bound) is not needed in either case here because Integer and Boolean cannot be subclassed nor you don't want any of its super classes (Number, Object).

The lower bound of CharSequence on the other one makes sense because you want to allow subclasses like String, SpannableString, etc.

Each of those statements is false. If you don't care enough to read why they are false after I went to great effort to explain why it makes sense to have ? super Integer, and choose to keep repeating false information you have definitely lost a great deal of credibility with me.

Bottom line is the signatures I posted in the request are correct and both should have "? super" and that has nothing to do with whether the classes are final or not.

from rxandroid.

Oh, and I do agree on the toString != CharSequence.

from rxandroid.

Yeah I assumed it was upper bound not lower bound.

you have definitely lost a great deal of credibility with me

This means nothing to me and is a useless comment here. Having to deal with triaging hundreds of issues a day it should be passable to understand why I don't read most of the content and always assume you don't know what you're talking about.

from rxandroid.

PR here: #85

from rxandroid.