Refactor to object oreinted standard?


#1

Hey there developers, I’m new to QT, and obviously new to this program as a developer. I’m at a conundrum as to which direction to go with this code. I’d like to work on it, but feel it needs refactored. Not trying to step on anyone’s toes… appreciate everyone’s hard work.

I’ve been reviewing the code for a week now. My original intent was to add 3 point curve functionality. Thinking add a file in the folder “curves” with the appropriate curve object, Add a line or two in the UI to add the 3 point curve button, etc… a week at most of an evening.

Maybe I’m looking at things wrong, but it confusing to me.

I’m from an object oriented programming background and was wondering what others thought. I’m not familiar with QT, but it seems that when you get past the UI, the bulk of the program is in two files. Is this because of limitations in QT, or is this a sign of an older program in c++ when it wasn’t truly OO?

What do others working on the code think?


#2

You’re not alone in your assessment of the code base.
It definitely needs to be refactored.


#3

Thanks for the response… I may take it on…experimenting with QT so that I am familiar with the system setup. What is the latest version of QT that would be reasonable to move up to? I’d like to work in the latest version, but would not want to leave everyone in the dust.


#4

Are you coming from a C background?

C++ is usually considered a “multi-paradigm” language. That is, you can use it for object-oriented, procedural, and even functional programming.

After learning Qt/C++… I can tell you that a lot of things are going to trip you up until you learn both as well as digging through all the “spaghetti code”. Plus I hope you’re already familiar with using a git client or that’s another thing to learn. I would suggest to take a look at many of the Qt examples… they really help grasp how to use the Q classes as well as the signals & slots mechanism. I also recommend to follow the Qt widget naming convention… like using a combobox name firstDayComboBox not comboBoxFirstDay. The Valentina code was done using the later and IMO making it harder to read. Also use full names - do not abbreviate. Here’s a BAD name example used in the code… GetPPath()… well what the hell is PPath? Printer Path? Pattern Path? Piece Path?

And No… the code is not is just 2 files. Much of using Qt (and thus Seamly2D) is connecting signals & slots… For example - say within the UI a widget is created for a 2 point line… that widget has signals associated with it - whether ones in the base class or ones added in a derived class. Those signals are then connected to a function/method (in another class) to be executed when say it’s clicked. There may also be a signal associated with the widget that is emitted when the function/method is executed and other widgets connected to that signal will then execute a function/method within it’s class. In other words… you may click on a widget, it does something, and at the same time updates another widget (or more).

BTW… you also have to have an understanding of how the schema works to add any new functionality to the program. The schema has to have elements added to identify a new object and it’s properties that are used to read/write the pattern, measurement, & label files. The schema is also used in another part of the program to validate the program version used… that is if you add a new 3 point line, an older program version can’t open & read the new 3 point line object.

Oh… and you also have to add the visualization stuff for the added tool… which means you need to understand how to work with the QGrahicsView and QGraphicsScene class functions. Also the new tool will need a UI dialog and you will need to know how to fill the various comboboxes with the object data… as well as the tool option creator in the PropertyExplorer section to be able to edit the object’s properties.

Almost forgot… you also have to setup undo/redo functions and push them to the undo stack.

Sounding rather daunting eh?

After 5 months or so of plowing through the code - I agree. I find it tedious. It’s like let’s throw a bunch of stuff against the wall and see what sticks. There’s code that just seems redundant - for ex: the (misnamed) tool option dialogs use the Creator UI, while the Tool Options docker uses the PropertyExplorer library…??? IMO pick one way or the other. Also variables, functions, classes, etc are sometimes poorly named (obviously due to the language barrier) making things confusing, and overall the code does not always follow a standard naming & style convention. I also find the whole folder structure horrendous. Going more than 3 folder levels down is BAD… not to mention there are resource files in no less than 3 separate places? Why? Unfortunately moving stuff around is not easy… you have to change any references, re commit, and edit .pro & .pri files using Creator or manually.

I also find it frustrating that many of the components of the program are not fully thought out from the start… which leads to “let’s throw more crap at the wall to make it work.” I ran Doxygen to get a handle on the classes and found the same thing with some derived classes being more than 3-4 levels deep - which means it’s harder to read & follow the code, and the base class probably wasn’t thought out well enough.

It’s almost like Roman made it intentionally hard to add or make changes to the code. :frowning:


#5

As per the doc on the git repo:

Qt 5.7.0 or later https://www1.qt.io/download-open-source (included in Qt install). I’ve been using 5.9.1. and it builds fine.


#6

@Douglas, I do not disagree with anything you said about the code. If you or anyone else has the time, inclination, and patience to create a new open source tool based on Qt and written in C++ with some naming conventions from the start, I am willing and able to contribute.

I am capable of making “patches” to spagetti code, but not willing to put those up for public use because of the daunting task of regression testing to insure that I did not break something.


#7

I have a limited amount of time and energy that I can devote to this project and I strongly believe in the motto “first, do no harm”


#8

In hindsight, I tend to agree with you. What we need is a complete rebuild. This code is really a mess, and it crashes alot. The Qt company said that it would provide an analysis and documentation of how it is built, the architecture, and use of widgets for $8000. Which is a deal. But I don’t have $8k.


#9

Ouch!!! That’s really sore :sob:


#10

If it’s being rebuilt from the ground up, how about using wxWidgets instead of QT?


#11

Wild idea: instead of rebuilding from scratch, how about taking FreeCADs 2D parametric drawing part as a foundation?


#12

Hi @martijnthe! Engineering CAD code isn’t easily updated to support patternmaking’s stages of development (1.drafting/drawing, 2. detailing/finalizing, 3. layout/marker, 4. print/export) even though it supports parametric design.

But if you’ve looked at FreeCAD code and think it can be done, please let us know!!! :smiley: