JakeWharton / RxBinding

RxJava binding APIs for Android's UI widgets.
Apache License 2.0
9.69k stars 972 forks source link

Feedback: README confusion about proper verb forms (and typo) #439

Open hk0i opened 6 years ago

hk0i commented 6 years ago

This is more of a linguistic nitpick and feedback about the readme doc but I think it might help the documentation clarity, since I have a bit of confusion about the description here. Also some of the points I mention may affect method names in future releases.

From the README:

Observable factory method names is the plural of the verb (e.g., click --> clicks()). The verb should be in the present tense, regardless of the platform's use (e.g., selected -> selection). When there are multiple versions of the same verb, prefix with a qualifying noun or adjective that differentiates (e.g., click vs. long click, item selection vs. nothing selection).

First, there is a typo here, it should read:

... method names are the plural.

But that doesn't seem accurate. What does it mean that the method names take the plural form of the verb? Clicks is the 3rd person singular form of the verb: he clicks, she clicks, it clicks, etc. The plural form would read: we click, they click. Clicks is also the plural noun form of the word click.

The part where selected -> selection has me a bit confused also, we've switched from a verb form to a noun, if we wanted the 3rd person present form of select it would be selects, no? I tried to find some code usage of this but I couldn't find anything in the docs except for RxView::selected() which is deprecated in favor of RxView::setSelected(), which disagrees with the style stated in the document.

This is all relatively minor so I'm not trying to make a huge thing out of it, but I think there are some inconsistencies here with the documentation and the actual style. It also looks like instead of a verb the pattern might actually just be the plural noun style? With the exception of this selection bit.

liminal commented 6 years ago

But that doesn't seem accurate. What does it mean that the method names take the plural form of the verb? Clicks is the 3rd person singular form of the verb: he clicks, she clicks, it clicks, etc. The plural form would read: we click, they click. Clicks is also the plural noun form of the word click.

Yes, I would say that the current naming is actually using plural nouns. The factory methods mentioned return Observables of events which can be seen as a group of emissions which in turn means they are named after the plural of the emitted noun. So an observable of click-events is created by a method called clicks. An Observable of a query text change is created by queryTextChanges.

Which (to me) is completely reasonable. In this case I think it's the README that gets the terminology wrong. No biggie but could be cleared up.

hk0i commented 6 years ago

Thanks! That makes a lot more sense actually. I'm still relatively new to Rx in general so I'm trying to get a handle on how some of these ideas connect and understanding the underlying reasoning behind these semantics helps greatly.

JakeWharton commented 6 years ago

I remember writing this in a hurry because people were contributing methods with wild names. I don't think I proof-read it and just pushed it up and then closed a few PRs pointing to the README. I would welcome a PR to correct whatever I got wrong! Otherwise I'll get to it eventually.

On Wed, Mar 28, 2018 at 5:47 PM Gregory McQuillan notifications@github.com wrote:

Thanks! That makes a lot more sense actually. I'm still relatively new to Rx in general so I'm trying to get a handle on how some of these ideas connect and understanding the underlying reasoning behind these semantics helps greatly.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JakeWharton/RxBinding/issues/439#issuecomment-377048503, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEZbtt8MkzQkOVpHftRrmjHk44geYks5tjAT-gaJpZM4S91lW .

hk0i commented 6 years ago

I might do that if I can free up some time over the next few days 👍