[Discussion] ActionCreators in v0.10.0

[tmtron]

Since v0.10.0 we must manually define the action creators and matching reducers:

NotesAction:

    @ActionCreator.Action(NotesActions.ADD_ACTION)
    Action add(int id, String content);

NotesReducer:

    @Action(value = NotesActions.ADD_ACTION,
            from = NotesActions.class,
            generateActionCreator = false)
    public List<Note> add(List<Note> state, int id, String content) {...}

This is much more work than before and redundant: e.g. now the parameters for the action must be defined in 2 methods (which have the same name) and referencing the action is also cumbersome (you need the constant and the java-class)

The motivation (according to the release notes) is :

Actions can be "shared" between multiple reducers. One reducer can handle actions from multiple Action creators.

Alternative

I think these goals could have been more easily reached, by doing this (relative to v0.9.3):

• leave the generated ActionCreator class as it is, but create it in a separate file (not as nested class in the ReducerImpl)
• introduce a new annotation: e.g. @ActionRef(SomeOtherAction.ACTION_CONSTANT) which does not create a new method in the ActionCreator (in comparison to the @Action() annotation)

Am I missing something here?

[Yarikx]

Pros and cons

Indeed using new approach requires defining actions explicitly. It was done on purpose for few reasons:

• Separate actions from reducers (So code that dispatches the action do not know the reducer that will handle it).
• Actions grouped together become an API, and as it's interface, can be easily mocked for tests.
• Reduce the amount of generated code directly used in source code. Nobody likes to compile the project twice just to have autocompletion. And refactoring becomes a pain.
• Share actions between reducers.

However, there are some downsides to this approach:

• As you mentioned, that a looks like 'duplication' of code (for action and reducer)
• A bit of reflection (Class.forName once per interface) is used to instantiate generated class.

Measuring pros and cons I can sacrifice few lines of code spent on actions to have more benefits. And I think it's semantically more close to what is considered a good practice in Redux community (defining action creators).

About your alternative:

As I think it's a better approach than we had in 0.9.3, it solves only one problem with the previous approach -- sharing action creators. However, as 'shared' action should be generated at least once in some reducer, it creates a notion of 'master' reducer for the particular action, which is not always true sometimes.

Imagine this example: We want to have action LOGOUT which will be handled in most of the reducers to 'reset' state to some initial state. Now we need to define this action in one reducer (let's say FoobarReducer) and reuse it in all the other reducers. So the client code should execute FoobarActionCreator.logout(), making this action semantically belong to FoobarReducer. But this is not true.

Furthermore, if developer decides to remove particular sub-state (with FoobarReducer reducer), the action will be still there (because all the other reducer can handle it), but we will have to replace usage of FoobarActionCreator to SomeOtherReducerActionCreator

0.10.0

The previous approach is still available and is turned on by default with generateActionCreator flag in @AutoReducer.Action for a smooth migration. I will leave it there for a couple of versions (but will switch it off by default) and will think about improving it.

[tmtron]

Thanks for the detailed response.

I think that the vast majority of actions in a project will be semantically related to exactly one reducer (e.g. User: ADD_USER, DELETE_USER, RENAME_USER, Notes: ADD_NOTE, ..) and there may be only a few edge cases (e.g. you mentioned logout') where this is not true. I hope we agree on that, because this is the basis for my reasoning.

I do agree that the ActionCreators should be separated from the Reducers, and I think we can mix both of our approaches to get the best of both.

Edge-case Example

magine this example: We want to have action LOGOUT which will be handled in most of the reducers to 'reset' state to some initial state. Now we need to define this action in one reducer (let's say FoobarReducer) and reuse it in all the other reducers. So the client code should execute FoobarActionCreator.logout(), making this action semantically belong to FoobarReducer. But this is not true.

When we mix our approaches, you can still create your own ActionCreator class manually

@ActionCreator
public interface UserSessionActions {
    String LOGIN_ACTION = "LOGIN";
    String LOGOUT_ACTION = "LOGOUT";

    @ActionCreator.Action(UserSessionActions.LOGIN_ACTION)
    Action login(String userName);

    @ActionCreator.Action(UserSessionActions.LOGOUT_ACTION)
    Action logout();
}

and then use @ActionRef to refer to it.

@ActionCreator
public interface SomeReducer implements Reducer<SomeState> {

    @ActionRef(UserSessionActions.LOGOUT_ACTION)
    public SomeState logout(SomeState state);
}

Normal example

We could introduce a way to specify the name of the generated action-creator class on the reducer: e.g. @AutoActionCreator:

@AutoReducer
@AutoActionCreator("NoteActions")
public abstract class NotesListReducer implements Reducer<List<Note>> {
    @Action("ADD_NOTE")
    public List<Note> add(List<Note> state, int id, String content) {}

    @ActionRef(UserSessionActions.LOGOUT_ACTION)
    public List<Note> logout(List<Note> state) {}
}

should generate this class:

@ActionCreator
public interface NoteActions {
    String ADD_NOTE_ACTION = "ADD_NOTE";

    @ActionCreator.Action(ADD_NOTE_ACTION)
    Action add(int id, String content);
}

which will in turn result in another generated class: NoteActions_Autompl.

Other reducers could use the generated action as usual:

@AutoReducer
public class FoobarReducer implements Reducer<SomeState> {
    @ActionRef(NoteActions.ADD_NOTE_ACTION)
    public SomeState doSomething(SomeState state) {}
}

Refactoring

Furthermore, if developer decides to remove particular sub-state (with FoobarReducer reducer), the action will be still there (because all the other reducer can handle it), but we will have to replace usage of FoobarActionCreator to SomeOtherReducerActionCreator

When you later decide that you want to get rid of the NotesListReducer, you can just move the NoteAction class from the generated folder to your sources, delete the NotesListReducer and everything will still be working.

What do you think?

[Yarikx]

Ok, now I got your point :)

So as I understand the point is to generate @ActionCreator interfaces (and implementations) for cases when action is 'bound' to a reducer.

I think this approach can be a better replacement of what is generated with generateActionCreator=true. And in fact, an additional annotation is not even required, as now you already can 'reference' action creators with from parameter of @ActionCreator.Action per each action.

This is something I need to think about for a couple of days, but in fact, it looks promising as a replacement for old generated ActionCreators.

As for myself, I see this feature more like quick 'bootstrap' tool to help to prototype and come up with state and reducers structure, but I will still recommend defining action creators explicitly for reasons provided above, e.g.:

• Heavy usage of generated code in sources,
• API will not be defined in source code directly (hard to explore the project w/o IDE, compiled code)

@tmtron Thanks for your contribution and this idea. I'll try to prototype and play with it this week.