API improvements

[ahmednfwela]

this PR uses polymorphism instead of multiple enum values to determine the swiper state
also fixes some issues where the timer would still run after the widget is disposed

[SalahAdDin]

@ahmednfwela It will require to enforce the types.

@TheAnkurPanchani Can you review this please?

[ahmednfwela]

@SalahAdDin which types exactly ?

[SalahAdDin]

You put final, but you don't define the variable type.

Can you avoid the component duplicate child when it is only one?

[ahmednfwela]

where exactly am i doing that ?

[ellie-me]

I had a look at your PR, I am not the owner of the repo, just to point out that there are a few code smells in your PR with concepts such as using final without declaring the type, lazy use of Object, creating variables in memory where they are not needed (don't duplicate), you get it, the devil is in the details. Instead, throw as much stuff as possible directly to the render functions, it is a good way to go but it seems it has breaking changes and it needs some linting, you want to also avoid breaking changes with this library as people will not migrate because this library is a clone of another and there are many forks of the same library.

To be fair, I believe that the best thing to do with this carousel library is to start over again with better design patterns, inheritance won't solve everything.

[ahmednfwela]

@eli-me what exact lines am I using final without declaring the type? or using Object ?

[ellie-me]

@ahmednfwela I have to do a full code review to point it out because your code is big, I am a bit busy because I am at work now but I might be able to have a look at your changes and do your code review plus make some amendments later on. It is possible you can give me write access to your PR?

[TheAnkurPanchani]

This PR has some conflicts. Please resolve them. I will have a look at all changes, but it will gonna take time as it has lot of changes.

[ahmednfwela]

conflicts resolved @TheAnkurPanchani

[ellie-me]

@ahmednfwela @TheAnkurPanchani hey I am still fighting with this library, but I believe I have to do a separate pull request.

Thing is, I tried to refactor the entire library for readability and portability and it turns out the way it was designed is as three separate modular libraries, yet, the original developer never did code splitting, they chunked all classes in big files and it's not really modular because they all depend on each other and I am thinking what way should we go. Basically maintaining this repo means we have to maintain the package flutter_page_indicator and transformer_page_view which by the way were not maintained anymore (3 years)

@ahmednfwela I will not touch your PR, just make sure it doesn't have any conflicts and merge it into master, I will continue with the refactoring on a separate thread after your contribution.

[ellie-me]

I forgot to mention @ahmednfwela that you put changes on all files in the library, including the transformer page view and the page indicator which are supposed to be different libraries. People which depend on the base packages (not the slider) and use this library will have an error for redefinition, but I think as well that it's not the case since most should be using a non-null version of this library like card_swiper or the other clones in pub.dev.

[ahmednfwela]

the general idea of this pr was to solve problems I was facing and weird exceptions

but it just evolved to be a refactor, and I just made a pr of it.

if there is no intention for this to be merged, it's ok, I will just continue referencing my fork

[ahmednfwela]

and I can't find anywhere where you say I put final without defining type or using Object

[SalahAdDin]

and I can't find anywhere where you say I put final without defining type or using Object

I mentioned you many different places where you didn't put the type.

[ahmednfwela]

do you mean as
final x = 5;?

[SalahAdDin]

These are just some examples about we commented.

[SalahAdDin]

Here you are removing the variable type.

[ahmednfwela]

1. this is the example package
2. value type is inferred from usage, List<T> values means that value is of type T, I fail to understand how that is a problem

[ahmednfwela]

also check: https://dart-lang.github.io/linter/lints/avoid_types_on_closure_parameters.html

[cedvdb]

final without type is not a code smell. Flutter team use that but that does not make not using it a code smell