Add a new abstract ListCubit that helps remove a lot of duplicate code

svthalia / Reaxit

The latest ThaliApp built on Flutter.

GNU General Public License v3.0

11 stars 3 forks source link

Add a new abstract ListCubit that helps remove a lot of duplicate code #445

Closed JAicewizard closed 1 year ago

JAicewizard commented 1 year ago

Closes #444.

Summary

A lot of ListState cubits share a lot of code, and might behave in slightly different ways in some circumstances (like, is it cached, does it propperly do some checks). This PR introduces a class that unifies all common behavior but allows the necessary freedom.

How to test

Steps to test the changes you made:

Visit all the pages that use a ListState

JAicewizard commented 1 year ago

@Scarletto (and maybe @DeD1rk since its originally his issue) could you take a look at this? Things that need to be done still

have a DoubleListState for 2 way lists (easy)
have a SingleListCubit that implements ListCubit for one-way lists (easy)
implement moreUp (easy)
implement emiting new state (easy)
use this in the cubits (takes time)

DeD1rk commented 1 year ago

I'm not sure there's any value in making a generic two-way version, as the calendar is the only usecase I can think of. Without doing any two-way support, the generic cubit can be very clean (I see there's already some 'up' logic in there that shouldn't be necessary)

DeD1rk commented 1 year ago

Oh ine more thing, with dart now having sealed classes, I think it would be very nice to now use those for ListState. I gave that a try already in that PR I never finished, but back then I couldnt profit from sealed classes yet, so also no pattern matching in the frontend

JAicewizard commented 1 year ago

I'm not sure there's any value in making a generic two-way version, as the calendar is the only usecase I can think of. Without doing any two-way support, the generic cubit can be very clean (I see there's already some 'up' logic in there that shouldn't be necessary)

Yeah I know, but a one-way SingleListCubit can easily implement this and just do nothing for upX. That's very easy and doesn't require much code at all.

JAicewizard commented 1 year ago

@DeD1rk Do you have any objections to this being merged? I can make a switch to composition instead of inheritance, but I think that for anyone new to the codebase it would be easiest to implement a cubit with inheritance (as the class where you write the code, is also actually the qubit).

DeD1rk commented 1 year ago

Seems nice, I'm starting to understand how it works now.

Some small improvements in naming and docs would be nice though:

supressDown: is a bit of a vague name. I'm not sure exactly what it means but maybe something like 'canLoadMoreDown'
most comments cpuld be doc comments, so they show up in an IDE when implementing cubits