leancodepl / leancode_cubit_utils

5 stars 0 forks source link

Add default mapper in `BaseQueryCubit` #23

Open pdenert opened 6 months ago

pdenert commented 6 months ago

In many situations QueryCubit does not need to map the returned data, it just needs to pass it on. So we can cover this situation with the default mapper to avoid writing this boilerplate code:

@override
T? map(T? data) => data;
mkucharski17 commented 4 months ago

I agree with @pdenert. I saw many times map implementation without any mapping.

michalina-majewska commented 3 months ago

@pdenert @mkucharski17

The default implementation can be used only when TData == TOut (so we can pass on the data value). I'd say it's already provided by SimpleArgsQueryCubit and SimpleQueryCubit as they both have TData == TOut and TOut map(TOut data) => data;.

My suggestion is, we start using extends on SimpleArgsQueryCubit and SimpleQueryCubit instead of ArgsQueryCubit and QueryCubit when TData == TOut. One difference is that we have to pass the request function as a constructor argument instead of overriding it.

Did I miss a case in which this doesn't work?

zltnDC commented 2 months ago

SimpleCubits were developed for hooks. We can extract them from use_query_cubit.dart and change the parameters to named parameters for better readability. Also, mention them in README.md.

michalina-majewska commented 2 months ago

Okay, will do

mkucharski17 commented 2 months ago

TBH I feel like I do not have enough knowledge for this discussion. I have never used this package. I just wanted to say, that I saw many times in Activy where @pdenert uses this package occurrences of the map method which does not do any mapping like code below.

T? map(T? data) => data

It seems like redundant code that could be implemented as a default behavior. I hope @pdenert can answer your questions @michalina-majewska.

pdenert commented 2 months ago

Yeah, usage of the Simple*QueryCubits will work for me. Changes suggested by @zltnDC also ‘ll made it more readable, but we need to keep in mind that, this will be a breaking change, so the version should be appropriately bumped