marcglasberg / async_redux

Flutter Package: A Redux version tailored for Flutter, which is easy to learn, to use, to test, and has no boilerplate. Allows for both sync and async reducers.
Other
234 stars 40 forks source link

Setting state's variable to null #83

Closed kkizlaitis closed 4 years ago

kkizlaitis commented 4 years ago

Good afternoon!

I have taken the TodoState from the repository's example and added a variable named selectedTodoId (which can be null):

class TodoState {    

  final List<Todo> todos;
  final int selectedTodoId;            

  TodoState({this.todos, this.selectedTodoId});

  TodoState copy({List<Todo> todos, int selectedTodoId}) {
    return TodoState(          
      todos: todos ?? this.todos,
      selectedTodoId: selectedTodoId ?? this.selectedTodoId);
  }             

  static TodoState initialState() => TodoState(todos: const [], selectedTodoId: null);
}

What is the best way to set copy the state by setting selectedTodoId to null? If I call the state's state.copy(selectedTodoId: null), the previous selectedTodoId is assigned (well, because Dart interprets as if selectedTodoId was simply not passed.

I may call TodoState.initialState().copy(todos: previousState.todos, selectedTodoId: null), but if I later add some more variables to the TodoState, I will most definitely forget about adding them to that call (which will lead to regressions).

What is your take on this problem, @marcglasberg ? :) Thank you!

marcglasberg commented 4 years ago

That's a problem with the copy pattern in Dart. There are many solutions, none of them perfect. I'll give you 2:

The following works because selectedTodoId can't be -1:

TodoState copy({List<Todo> todos, int selectedTodoId = -1}) {
    return TodoState(          
      todos: todos ?? this.todos,
      selectedTodoId: (selectedTodoId != -1) ? selectedTodoId : this.selectedTodoId);
  }      

The following is a special method just to remove the selected todo:

TodoState withoutSelectedTodo() {
    return TodoState(          
      todos: todos,
      selectedTodoId: null);
  }      
kkizlaitis commented 4 years ago

Thanks @marcglasberg for your suggestions!

TodoState withoutSelectedTodo() {
    return TodoState(          
      todos: todos,
      selectedTodoId: null);
  }

This suggestion doesn't do much because you still have not to forget to add a parameter for new variable in the TodoState.

TodoState copy({List<Todo> todos, int selectedTodoId = -1}) {
    return TodoState(          
      todos: todos ?? this.todos,
      selectedTodoId: (selectedTodoId != -1) ? selectedTodoId : this.selectedTodoId);
  }

I like this more because it is less prone to regressions. Unless your argument can be negative :) Also, what would you do if the argument was a String? Define a "NO_VALUE" private variable that user would hopefully never use?

marcglasberg commented 4 years ago

I'd use an empty string. If you don't want to differentiate between null and empty string.

Or I'd use a combination of characters from the private use areas: https://en.wikipedia.org/wiki/Private_Use_Areas