Refactoring needed - extremely large files/classes
Issue #404
new
I’ve been taking a look at the FlatCAM source to see if I can contribute to the project.
The first thing that strikes me is that there are a number of extremely large, multi-thousand-line files. There are also many files that contain multiple unrelated classes that would be better off split into multiple files (for example, FlatCAMObj.py). Also, there are god objects that do far too much (FlatCAMApp). Decomposing these into smaller components would improve the maintainability of the system.
Would anyone object to me doing this sort of thing?
Comments (4)
-
-
Ok David @David Robertson ,
I’ve replaced the isinstance(obj, FlatCAMxxxxx) with if obj.kind == ‘etc’ everywhere and in the process I’ve refactored the FlatCAMObj. The classes from FlatCAMObj are now each in it’s own file in the flatcamObjects folder.
The Tcl shell related methods are now moved in the flatcamTools.ToolShell file.If you are working on the Beta branch I would appreciate any refactoring on the FlatCAMApp.App class.
Thanks,
Marius -
reporter
Okay, I will take a look at that later. I will be working from the beta branch, yes.
David
-
reporter
I’ve put in a pull request for my first lot of changes surrounding prefs/defaults: https://bitbucket.org/jpcgt/flatcam/pull-requests/306/extract-prefs-code-from-app-class/diff
- Log in to comment
Hi David,
It will help to mention on which branch are you willing to work on.
I think that this is a good idea, I was doing this myself, slowly, in the Beta branch.
Some time ago I started to move all the parsers into their own files and own folder. In the recent push that I made to beta branch I started to work on moving all the Tcl Shell related methods out of the FlatCAMApp class to the flatcamTools.ToolShell file.
I also moved all the slot methods (that are connected to qt signals) related to the Preferences tab out of FlatCAMApp to flatcamGUI.PreferencesUI. This file is quite big itself and due for fragmentation itself.
FlatCAMObj was on the line but did not do it yet until I actually manage to get rid of all the isinstance things that point to one of the other classes inside that file.