Evaluate ViewBag Helper

[JakeWharton]

Frequently an action needs applied across multiple views. Currently I've been writing standalone methods for this which is cumbersome and prone to error long-term.

@InjectView(R.id.first_name) EditText firstNameView;
@InjectView(R.id.middle_name) EditText middleNameView;
@InjectView(R.id.last_name) EditText lastNameView;

private void setNameEnabled(boolean enabled) {
  firstNameView.setEnabled(enabled);
  middleNameView.setEnabled(enabled);
  lastNameView.setEnabled(enabled);
}

setNameEnabled(false);

A view bag aggregates multiple views of type T and allows applying properties across all of them at once.

interface ViewBag<T extends View> {
  void <V> apply(Property<T, V> property, V value);
  List<T> asList();
}

@ViewBag({R.id.first_name, R.id.middle_name, R.id.last_name})
ViewBag<View> nameViewBag;

nameViewBag.apply(View.ALPHA, 0);

You can define your own properties for common actions.

final class Views {
  public static final Property<View, Boolean> ENABLED = ...;
}

nameViewBag.apply(Views.ENABLED, false);

Generics allow using non-View-specific methods.

final class TextViews {
  public static final Property<TextView, Typeface> TYPEFACE = ...;
}

@ViewBag({R.id.first_name, R.id.middle_name, R.id.last_name})
ViewBag<EditText> nameViewBag;

nameViewBag.apply(TextViews.TYPEFACE, customTypeface);

[swanson]

Could you just use varargs?

ViewBag.disable(firstName, middleName, lastName);
ViewBag.applyFont(customTypeface, firstName, middleName, lastName);

View[] nameFields = [firstName, middleName, lastName];
ViewBag.disable(nameFields);
ViewBag.applyFont(customTypeface, nameFields);

public class ViewBag {
    public static disable(View... views) {
      for (View v : views) {
          v.setEnabled(false);
      }
    }

   public static applyFont(Typeface typeface, View... views) {
      for (View v : views) {
          v.setTypeface(typeface);
      }
   }
}

Or is the big win the grouping in the same place as your injection?

[JakeWharton]

@swanson Yes but that has two major downsides that I can see: it requires an allocation of an Object[] on every invocation and it requires a separate method implementation for every property you want to control.

[swanson]

Allocation - yup, no way around that I suppose.

Enumerating the properties - true, but wouldn't the library handle all common view properties? If you want something custom, you write your own 3-line static method vs defining a custom Property.

[JakeWharton]

I thought of another downside: It suffers from having to duplicate the list of views multiple times. When I add a Mr./Ms. selection view I have to update the list that's passed to enable/disable/etc. at every call site.

The problem with common is that I can't really know what the common ones are. Some are obvious, but sprinkled throughout my codebase I have one-off batch applications of strange properties like color, translation, or playing an animation. By making the mechanism of applying the property to multiple views (ViewBag) independent of the actual application (Property) it allows every call site to behave the same while still being extensible.

[swanson]

I don't follow the Mr./Mrs. example.

@InjectView(R.id.title) EditText titleView;
@InjectView(R.id.first_name) EditText firstNameView;
@InjectView(R.id.middle_name) EditText middleNameView;
@InjectView(R.id.last_name) EditText lastNameView;

private View[] nameFields;

public void onCreate(Bundle b) {
    super.onCreate(b);
    nameFields = [titleView, firstNameView, middleNameView, lastNameView];
}

public void someThingHappened() {
    ViewBag.disable(nameFields);
    ViewBad.colorize(R.color.gray, nameFields);
}

public void wizzBangPow() {
    ViewBag.enable(nameFields);
    ViewBag.fadeIn(2000, nameFields);
}

(Aside: this example set of controls should probably live in a custom view not an activity, but bear with me)

[JakeWharton]

Ah, but now you've changed what's happening to start solving the problems and you are moving toward what my implementation of ViewBag would be! :)

So let me throw another use-case into the mix. Often times I have a set of buttons on screen which I want to control. In code, I only interact with these buttons through @OnClick-annotated methods. In order to control properties on them, though, I have to add @InjectView fields of them all.

In your new example I would list the injects and the array as fields, perform injection, create the array and pass in the list of fields, and then at the call site use that array.

I think it's getting better, but I still have to duplicate the list of fields twice. Losing the allocation of the Object[] is good. I don't know if I'm a fan of having to write a method for every single action that I want to perform and having it have to know about iteration. My goals here are to eliminate duplication and provide an extensible means of applying things to these groups of views.

[swanson]

Yeah - I think we are converging on the same solution. Good discussion, IMO.

Extensibility is probably a matter of preference in this case. Personally, I prefer the explicit methods but, hey, maybe that's just me :) Reminds me of fest-android in that sense: a bit of work in the library to make explicit methods, but I think it is more readable than generic matchers.

At first glance, this seemed like it might fit better as its own library - applying properties to a group of views doesn't seem to have much to do with "injection" - but I hadn't considered the emphasis on grouping up views with the annotation. Maybe it still should be it's own thing - ButterKnife could provide the grouping, but ViewBag could handle applying properties (and be used separately if you want to just pass an array like in my example).

[JakeWharton]

Yeah it's a tough distinction to make. I like that you brought up fest-android. That's a good counter-point to what I've been saying. I've been thinking about something more like Hamcrest where you can create arbitrary matchers but perhaps there is value in being concrete. I'll have to think about it more.

As to "injection", I've been meaning to rebrand the library as view-based boilerplate reduction for a while now!

[EmmanuelVinas]

Be able to apply an animator to the ViewBag could also be useful.

[JoanZapata]

About the custom properties / explicit methods debate, I also think it's only a matter of preference, and custom properties don't prevent anyone from making his own library on top of it to provide explicit methods. So I actually think it's better.

Moreover it would avoid having both ViewBag and ExtendedViewBag in the project, depending on whether we need more than the default methods or not.

[bobz]

Here's a similar use case I ran into today, than I'm not sure ViewBag would handle. (Although it would help.)

    protected void updateViewsByCase(){
        if (caseButton()){
            button.setVisibility(View.VISIBLE);
            textView.setVisibility(View.GONE);
            progress.setVisibility(View.GONE);
        } else if (caseText()){
            button.setVisibility(View.GONE);
            textView.setVisibility(View.VISIBLE);
            progress.setVisibility(View.GONE);
        } else {            
            button.setVisibility(View.GONE);
            textView.setVisibility(View.GONE);
            progress.setVisibility(View.VISIBLE);
        }
    }

This may not be the best way to achieve what I'm looking for... but I do find myself writing this type of code occasionally. Here it's how I handle different cases in a ListView empty view.

[JakeWharton]

@bobz Yep. Still a bit awkward in that case but it'd become:

thingsBag.apply(Views.VISIBILITY, GONE);
if (caseButton()) {
  button.setVisibility(VISIBLE);
} else if (caseText()) {
  textView.setVisibility(VISIBLE);
} else {
  progress.setVisibility(VISIBLE);
}

Although in general you might want to consider a tri-state value to simplify to:

Whatever whatever = determineCase();
button.setVisibility(whatever == FOO ? VISIBLE : GONE);
textView.setVisibility(whatever == BAR ? VISIBLE : GONE);
progress.setVisibility(whatever == BAZ ? VISIBLE : GONE);

[JakeWharton]

FYI opinionated people see #119 for the implementation of this.