Is this not a pure function?

Question

I am learning state management for web development, and come across this redux tutorial with the pure function as below. However the statement : "action.todo.id = state.todos.length + 1;" makes me suspect this pure function was ... IMPURE. Please enlighten me, thanks!

export function rootReducer(state: IAppState, action): IAppState {

    switch (action.type) {
        case ADD_TODO:
            action.todo.id = state.todos.length + 1;    
            return Object.assign({}, state, {
                todos: state.todos.concat(Object.assign({}, action.todo)),
                lastUpdate: new Date()
            })

        case TOGGLE_TODO:
            var todo = state.todos.find(t => t.id === action.id);
            var index = state.todos.indexOf(todo);
            return Object.assign({}, state, {
                todos: [
                    ...state.todos.slice(0, index),
                    Object.assign({}, todo, {isCompleted: !todo.isCompleted}),
                    ...state.todos.slice(index+1)
                ],
                lastUpdate: new Date()
            })
        case REMOVE_TODO:
            return Object.assign({}, state, {
                todos: state.todos.filter(t => t.id !== action.id),
                lastUpdate: new Date()
            })
        case REMOVE_ALL_TODOS:
            return Object.assign({}, state, {
                todos: [],
                lastUpdate: new Date()
            })
    }
    return state;
}

score 2 · Accepted Answer · edited Jun 20 '20 at 09:12

2

TL;DR - no it is not.

Let's examine the definition of a pure function. From wikipedia:

In computer programming, a pure function is a function that has the following properties:

Its return value is the same for the same arguments (no variation with local static variables, non-local variables, mutable reference arguments or input streams from I/O devices).

Its evaluation has no side effects (no mutation of local static variables, non-local variables, mutable reference arguments or I/O streams).

Although your function does conform to the second condition, using new Date() - makes it impure.

The reason for its impurity in your case is the fact that the date is different for each of the functions invocation - regardless of the parameters passed.

In order to make it pure, you should pass the date as an additional parameter which will allow you to have the same output for the same input.

Zaptree had also mentioned that mutating the length of your item IDs, i.e action.todo.id = state.todos.length + 1 is impure as it might affect other parties referencing to it.

edited Jun 20 '20 at 09:12

Community

1
1

answered May 15 '19 at 13:14

silicakes

6,364
3
28
39

Isn't "action.todo.id = state.todos.length + 1; " also side effect as it alter the input value? – T Luu May 15 '19 at 13:30
Although your statement is 100% correct, the new Date() code does not cause issues with redux. Mutating state is an issue because you can have unexpected side effects because of how redux optimizes performance by doing shallow comparisons to determine if a render needs to happen. Mutating state would be an issue but not something like this. – Zaptree May 15 '19 at 13:30
@TLuu - No it's not since state is passed as a parameter. if you'll pass down something like `{todos: [1,2]}` as the state - you'll always get 2 as its `length` property so you'll have the same result. As a side note - increasing the length property of a list, without actually adding an item to it - is just a bad practice. consider adding an item to the list instead - and assign the id to it during creation. – silicakes May 15 '19 at 13:33
@Zaptree - There's plenty to say about reduxes faulty assertions and the fact it forces immutability, however OP asked about the functions purity. Inferring it's part of Redux shouldn't matter for this answer. – silicakes May 15 '19 at 13:34
@silicakes I'm just putting in the comment for the sake of giving TLuu an explanation in the context that he is working in. Also your comment about it being ok to mutate parameters is not correct, it violates rule 2 that you mentioned. – Zaptree May 15 '19 at 13:40
@Zaptree - I'll accept that and will edit my comment. – silicakes May 15 '19 at 13:44
Thanks for the help, guys!! – T Luu May 15 '19 at 20:27
@TLuu - NP, please mark the relevant answer (the checkmark next to it) in order for people to know which of them had helped you the most. – silicakes May 16 '19 at 07:26

score 0 · Answer 2 · answered May 15 '19 at 13:26

You are very correct in realizing that the following code is impure:

action.todo.id = state.todos.length + 1;

The correct way to do this would be the following (using es5 syntax like you are):

var newTodo = Object.assign({}, action.todo, {
  id: state.todos.length + 1
});
return Object.assign({}, state, {
    todos: state.todos.concat(Object.assign({}, newTodo)),
    lastUpdate: new Date()
})

In your original code you are basically mutating the todo passed in from the action.

Is this not a pure function?

2 Answers2