Improve naming for actions

[mzorz]

Coming from here

Given that the general expectation about FluxC Store operations is for the store to take care of the fetching and storing, I suggest that we come up with a naming convention that suggests in a clear way when an operation is an exception to that rule, i.e. has no effects in the actual store (i.e. doesn’t store anything).

As pointed out here, this is the list of current actions that have no effect in the store:

IS_WPCOM_URL
SUGGEST_DOMAINS
FETCH_CONNECT_SITE_INFO
FETCH_WPCOM_SITE_BY_URL

The naming convention should also take into account for actions that are unauthed but affect the store, or authed actions that never affect the store.

My suggestion (proposal?) is to add prefixes or suffixes for the current action naming convention, like:

FETCH_X - request data from the server
PUSH_X - send data to the server
UPDATE_X - local change
REMOVE_X - local remove
DELETE_X - request deletion on the server

NS_FETCH_X - no store op - request data from the server but don't store
NS_PUSH_X - no store op - send data to the server but don't store

NA_<ACTION> - non-authed action

for example:
NA_NS_FETCH_CONNECT_SITE_INFO - non authed, no store op: fetch of site info

For the current list, this would be renamed to:

NA_NS_IS_WPCOM_URL
NA_NS_SUGGEST_DOMAINS
NA_NS_FETCH_CONNECT_SITE_INFO
NA_NS_FETCH_WPCOM_SITE_BY_URL

Thoughts welcome

cc @aforcier @oguzkocer @maxme @tonyr59h

[mzorz]

Also, consider media handling and rename UPLOADED_MEDIA to make it fit this scheme (see https://github.com/wordpress-mobile/WordPress-FluxC-Android/pull/533#discussion_r137307617)

[tonyr59h]

Also, consider media handling and rename UPLOADED_MEDIA to make it fit this scheme

I think it's a good idea to rename it, stick to the common convention. I also like the idea of the suggested prefixes, but they're a bit verbose and the info could be conveyed through documentation.

[mzorz]

they're a bit verbose and the info could be conveyed through documentation.

Agreed, I think abbreviated prefixes are supposed to make it not so verbose (or not so long at least). I'm more inclined to having a mnemonic rule rather than long names, and I certainly don't like the current ambiguity where you need to have a 2nd level of indirection to understand what something is doing or supposed to do when we could potentially read the docs just once and then make sense of things by just reading code, i.e. you should ideally immediately know what things are about

[maxme]

I agree we should rename UPLOADED_MEDIA to PUSH_MEDIA. I'm not sure about the prefixes, I don't think they are worth the added verbosity and not super obvious in usage: mDispatcher.dispatch(SiteActionBuilder.newNaNsFetchConnectSiteInfoAction(url))

[mzorz]

I'm not sure about the prefixes, I don't think they are worth the added verbosity and not super obvious in usage: mDispatcher.dispatch(SiteActionBuilder.newNaNsFetchConnectSiteInfoAction(url))

Gotcha, prefixes and abbreviations here don't help readability. What about a combination of both prefix and postfix? i.e. using NoStore prefix as it's a modification of the core functionality (fetch & store) and a postfix for modifiers of non-core functionality (like authentication):

mDispatcher.dispatch(SiteActionBuilder.newNoStoreFetchConnectSiteInfoAction_NoAuth(url))

to generalize a bit, this'd be the form: newNoStore<action_on_actionedObjectName>_NoAuth

What do you think?

[tonyr59h]

newNaNsFetchConnectSiteInfoAction

I'm sure this would force people to read the documentation 😆

Rather than adding this info to the action name (ie NA_NS_*) we could update the annotation parameters and present the info in the generated builders. Maybe another level of indirection like so:

mDispatcher.dispatch(SiteActionBuilder.NoStore.newFetchConnectSiteInfoAction(url))