Bloc: Equals of currentState and nextState by hashCode

Created on 23 Sep 2019  路  11Comments  路  Source: felangel/bloc

Is your feature request related to a problem? Please describe.
A problem with storing mutable objects in a State (list, map).
When we work with the currentState, we can change it if it contains collections.
Thus, we will change the currentState, which causes another problem, during the creation of a new state by changing the old we received two equivalent states
It鈥檚 clear that these are mistakes of the programmers, but I would like to somehow fix the situation and remove the possibility of a bug

Describe the solution you'd like
To solve the problem of "equivalent States", we propose that on each new state in stateStream, calculate and store its hash and compare the equivalence first by hashCode and then by equals

Describe alternatives you've considered
In your examples (todolist) you solve this by recreating the list. It works great, but 1) it can be forgotten, 2) it can be complicated when you have a complex structure (a model with a list of models with a map ...)

Additional context
We tried to solve the problem through the use of hashCode. You can see it here (changing in fork https://github.com/tvip/bloc/commit/41d735b85e7d5474e43ea34ba4aea17130474e4c, sample https://github.com/tvip/bloc_issue_sample/blob/master/lib/main.dart) However there was a flaw with the Transition object as it is created on the basis of already changed currentState

question

Most helpful comment

Ok, thanks @bigworld12 and @felangel! I will use immutable collections in my states

All 11 comments

having mutable states is bad design pattern, states should mainly be a notification for the UI, not carry actual data.
the mutable data should be stored in a repository, not get carried around in states.
check the architecture here : https://felangel.github.io/bloc/#/architecture

@bigworld12, i am making an intermediate state block like this (https://github.com/felangel/bloc/blob/master/examples/flutter_todos/lib/blocs/filtered_todos/filtered_todos_state.dart#L16)

In your example, you had this problem, but solved it like this (https://github.com/felangel/bloc/blob/master/examples/flutter_todos/lib/blocs/todos/todos_bloc.dart#L47). I used immutable states, but a collections can't be immutable(except *.unmodifiable)

you can have an immutable list
https://api.dartlang.org/stable/2.5.0/dart-core/List/List.unmodifiable.html
but if you want to use list anyway, just add a timestamp to the state, so that everytime you get the list, the date changes.

also note that the samples are just a quick guide on what you can do, they don't employ any design patterns

But I want to compare my states by data inside. Timestamp it is bad solution in my opinion.

what do you think of hashcode comparison?

globally, I think using mutable lists in a state is a normal situation in development, you just need to consider this in the bloc implementation. Would you disagree with this? use mutable lists in states is that wrong?

i do disagree, states should be immutable and equatable, they should be fast to create and carry as little data as possible.
if you have mutable/large data, put them in a repository and let the state describe the changes to these data.
don't forget that flutter works by rebuilding the UI each change, so if you are going to rebuild the ListView anyway, why not just refer to a single location containing the data, and the states should only notify the UI to be rebuilt.

@bigworld12, I'm fully agree with you, that all data in state should be immutable.
It is impossible in Dart to create immutable List/Map and check it at compile time.

List.unmodifible() will be checked only in runtime and it is problem. It is very easy to make mistake inside Bloc logic and to call currentState.list.add(xx) instead of List.of(currentState.list).add().

I have tried to find dart linter which can prevent modify List/Map inside classes, marked@immutable annotation but failed. There is no such linter.

Also we can wrap to list copy in the State constructor, like this:


@immutable
class MyObject extends Equatable {
   final String objName;
   MyObject(this.object) : super([objName]);
}

@immutable
class MyState extends Equatable {
   final List<MyObject> myObjects;
  MyState(List<MyObject> objs) : myObjects = List.of(objs), super([myObjects]) {}
}

But in what will happens if we will add other Container inside MyObject?

class MyObject extends Equatable {
   final String objName;
   final Set<OtherObject> otherObjects;
   MyObject(this.object,this.otherObjects) : super([objName, otherObjects]);
}

So, in this case we need to be sure, that OtherObject.otherObjects should be also copied in the constructor with Set.of()

Another problem, is that constructor generator in IDEA will generate basic constructor without copying of the containers, even if class is marked by @immutable

So, actually it is very easy to look over this situation during coding.

Suggested solution with storing hashCode of the state is great workaround and can handle such situations.

Hi @Mishanya840 馃憢
Thanks for opening an issue!

Regarding your question, bloc is built on the assumption that all state must be immutable. Your proposal to use hashCode to workaround mutating state is not a valid proposal because by definition:

Hash codes must be the same for objects that are equal to each other according to operator ==.
(source)

As it stands, bloc internally compares currentState == nextState. If that expression evaluates to true, then no state change will occur. By definition, if that expression evaluates to true then currentState.hashCode == nextState.hashCode must also evaluate to true.

Furthermore, correct me if I'm wrong but, your proposal would encourage mutating state rather than creating new instances of a state which will lead to lots of potential problems.

Closing this for now. If I misunderstood the proposal feel free to comment with additional information and I'm happy to continue the discussion 馃憤

Hi @felangel, I think we have a misunderstanding.
The idea is to save the hashCode when the State first appears in the block, and compare this hashCode with the hashCode of the new state.
Look at this, please https://github.com/tvip/bloc/commit/41d735b85e7d5474e43ea34ba4aea17130474e4c#diff-3f8ab4914c19c62cee0817498efb5cd0R141

"and compare this hashCode with the hashCode of the new state."
if the hash codes are equal, then the properties are equal.

Ok, thanks @bigworld12 and @felangel! I will use immutable collections in my states

Was this page helpful?
0 / 5 - 0 ratings

Related issues

shawnchan2014 picture shawnchan2014  路  3Comments

clicksocial picture clicksocial  路  3Comments

1AlexFix1 picture 1AlexFix1  路  3Comments

Reidond picture Reidond  路  3Comments

craiglabenz picture craiglabenz  路  3Comments