Where do layout_* attributes belong?

Where do layout_* attributes belong?
Cover image: “Meteors Show” by Edoardo Brotto — on flickr

tl;dr: in the styles or in the layout files? In the layout files.

As some of you might know, I work at an (awesome) mobile agency, Novoda. Rotating through several projects over the years is a core part of working in an agency. In my case, I’ve ranged anywhere between 4 and 18 months on the projects I’ve worked on (excluding a couple of short one-offs, like code reviews). As such I’ve only worked on an handful of codebases in my 3 and a half years here, but a great thing I love about Novoda is that everyone is encouraged to help out with code reviews and pairing with other projects from time to time.

This means that, all in all, I have probably worked on and looked at a dozen different codebases so far. Code of any size and age, from companies of every kind, internal and external dev teams, contractors and agencies.

Across all those project, a pattern emerged:

Almost nobody uses styles consistently.

Some don’t use them tout court (what a shame!), some use them in some cases but not in others, some put literally every attribute they can into the styles. What I do — and what we do in general at Novoda — is to leave all and any layout_* attributes in the layout file, and move all the other attributes (except android:id and android:theme) into styles.

You might argue that, like most codestyle matters, it doesn’t really matter where your attributes sit. That it’s all matter of personal preference.

I disagree.

Putting layout_* attributes in styles is generally discouraged.

There's several reasons for that, but the main one is that those attributes only make sense in the context of the parent ViewGroup parsing them to compose the LayoutParameters (hence the layout_ prefix in the attributes' names).

Note: the always awesome Nick Butcher pointed out to me that the equally always awesome Dave Smith has written something about this same topic three years ago. He goes into more details on this primary reason than I do; I recommend reading his post if you are interested.

The gist of this argument is that when you put those attributes in a style, you’re moving them away from the consumer of those attributes, which is the containing ViewGroup. All other attributes are parsed by the view they're applied to, but not the layout_* ones. So while it makes sense to have, say, android:textColor, or app:cardCornerRadius in a style, the same does not apply to layout_ attributes.

Note: when I say layout_ attributes, I mean both android:layout_ and app:layout_ attributes, since they’re effectively the same.

If you change a view’s parent ViewGroup from, say, FrameLayout to RelativeLayout, the android:layout_gravity attribute you have on it won't make any sense anymore since RelativeLayout doesn't use it to create the LayoutParameters for its children. If you have these attributes in the layout file, Lint will gladly point this issue out, both in the IDE and when running gradlew lint.

If that attribute were in a style you would most likely forget to check if it still applies, and Lint would not be able to help you either, because styles don’t have a context. There’s no way to know if an attribute you put in a style makes sense without figuring out the context in which it will be consumed — that is, the parent of the view(s) the style is applied to, and the view(s) themselves. This is why Lint doesn’t even try to sort that out: it’s too hard to do it correctly for it to be worth it, and it’s not necessarily valid or invalid all the time.

For example, if you have android:text in a style, and then apply the style both to an ImageView and a TextView, it will make sense for the latter but not for the former. This is rather complicated to infer for tooling because of how styles and layouts work. On top of that, it is near impossible to do properly for custom views since it’s not immediately clear which attributes are supported by which views.

The same goes for layout_width and layout_height too, which some think are a special case. In fact, in some cases they may not even be required; the ViewGroup might just be able to infer them from the other constraints they have.

Collateral damage

In addition to the above reasons, you should consider that the layout editor will not like it if you start moving layout attributes to the style. This is especially important when using ConstraintLayout, as it's meant to be mostly used in conjunction with the visual editor, instead of hand-crafting the XML.

While the editor will work fine if you have layout_ attributes in the styles, the moment you change anything it will duplicate (and thus override the styles’) attributes you’re touching into the layout file. That’s rather bad and can lead to confusion and bugs.

I also like to think the tooling is trying to show you the way

Lastly, you should consider that while layout_ attributes and the layout XML files inform the layout, all others attributes determine the appearance of the views in a layout. The former semantically belong together with the rest of the information in the layout file, while the styles should be the place where we define the appearance.

Now, you might or might not agree with this last reasoning. That’s up to you, really, but the important thing is that…

You put the layout attributes in the layout, and everything else in the styles!
Now go and make me proud

Thanks to Lorenzo Quiroli, Daniele Conti and Mark Allison for proofreading