https://summerofcode.withgoogle.com/projects/#5663079975616512
The refactoring is finished, all global state is now in one place where it can be easily moved to a ViewModel or retained headless fragment.
All singletons have been removed and most Context leakage has been fixed.
The CommandManager has been refactored and greatly simplified.
More than 200 local unit tests were added during the Google Summer of Code, the overall testability and reusability has improved as well.
All resources have been renamed to avoid any collisions with Pocket Code.
The actual transition into the Pocket Code directory has yet to be done.
- Catrobat/Paintroid#511 - Refactor the
MainActivity - Catrobat/Paintroid#515 - Refactor
DrawingSurfaceListener - Catrobat/Paintroid#516 - Refactor
PaintroidApplication - Catrobat/Paintroid#522 - Refactor
CommandManagerand layer menu - Catrobat/Paintroid#523 - Remove
Observer/Observablerelation between commands - Catrobat/Paintroid#524 - Refactor
ColorPicker - Catrobat/Paintroid#527 - Refactor
IndeterminateProgressDialog - Catrobat/Paintroid#529 - Refactor
MainActivityto a MVP architecture - Catrobat/Paintroid#530 - Rename resource files
- Catrobat/Paintroid#531 - Rename resource ids
- Catrobat/Paintroid#532 - Move fonts
The MainActivity was tightly coupled with a base class NavigationDrawerMenuActivity, none of which worked without the other. I therefore chose to merge these two classes. Further did the MainActivity catch configuration changes itself, breaking the Android activity lifecycle.
The constructor has been refactored and now complies to the lifecycle.
There were calls to AsyncTasks all over the place which made it hard to work with this class or even understand it. This has been refactored into classes, communicating with the MainActivity via interfaces (CreateFileAsync, LoadImageAsync and SaveImageAsync).
The DrawingSurfaceListener used to create a thread each time a user touched the screen and called functions on the DrawingSurface and tools. This lead to unpredictable behavior and crashes. This subclass is now using the handler to push itself on the queue asynchronously and will only call from the main thread.
There are local unit tests covering the behavior as much as possible.
The version number can be extracted via a BuildConfig variable which is automatically generated by gradle. This simplifies the About Dialog and removes code from the PaintroidApplication, which now only initializes some global variables. While at it I refactored the Terms of use and About dialog a bit.
The CommandManager functionality was scattered across at least three classes, using async functionality everywhere, using synchronize blocks in random places on random variables.
I moved everything state related in a LayerModel class, created a LayerContracts interface, describing all communication between sub-interfaces described in this contract.
Supplying this LayerModel to all Commands instead of only the currently active layer allows the commands to act way smarter and avoids many nasty previous hacks.
Using a Model-View-Presenter architecture and dependency injection allowed me to create unit tests for the new presenter, which handles all layer interactions, as well as more local unit tests for the commands.
All in all this greatly reduced the code used in production, while adding lots and lots of local unit tests.
All Commands previously derived from a BaseCommand class, which extended Observable, which was used by some tools, which all extended from Observable and implemented Observer. This created way more headaches than needed, the CommandManager now simply notifies the tools when a command finished executing, and the tool can update accordingly without the previously complicated architecture.
The ColorPickerDialog was tightly coupled with internal state in Pocket Paint. It was implemented as a Singleton, leaking Views and the Activity, which was previously solved with some nasty startup code.
I Refactored it to a AppCompatDialogFragment, which now also saves the selected tab and restores its state on configuration changes.
This was another singleton class, also leaking Context, with complicated startup code. I also refactored it to a AppCompatDialogFragment.
The MainActivity was only testable with integration tests, I moved all application logic in a presenter, described in a MainActivityContracts class.
All communication happens now through this contract.
Additionally a Navigator handles all dialogs and switches to another activity.
A Interactor now handles all asynchronous file I/O calls.
There are now > 100 local unit tests covering the application logic.
All resource files (layout files, drawables, values) have been renamed to be easily distinguishable from Pocket Code files (via prefix). Some drawables could be removed, as the functionality was actually long gone. The only native cpp file was renamed to a more fitting name.
All resource ids have also been renamed to be easily distinguishable from Pocket Code resource ids and to avoid collisions.
External fonts were previously in the assets directory. I was able to move them into the resource directory and adapted the tests to work with this change.