-
-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor drawing of elements: hide logic behind common interface #694
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Both ellipses and rectangles are drawn from a single object named "Area". Drawing logic looks from where it got the object: if it's from "rectangles" container, then draw a rectangle, otherwise draw an ellipse. This commit creates new "Ellips" object type, separate from "Area". It opens a way to: - hide a dedicated ellipsis rectangle drawing logic in a method associated only with "Ellips" objects - distinguish ellipses and rectangles not by their place of residence, but by their nature. The name "Ellips" without "e" at the end is used because "Ellipse" conflicts with a painting named that way, which is declared in the same namespace. I didn't want to go into any additional trouble and chose to go with this type of a "workaround"
Some classes have members of container types, storing "Ellips" objects, i.e. ellipses. Just assigning a proper name.
After extracting ellipse from "Area" to a separate class, "Area" means only a rectangle.
The goals of this are - add a generic API for qucs::Line, qucs::Arc, qucs::Rect, qucs::Ellips - hide drawing details from clients. If you take a look at code now you'll see that drawing of for example a line in Component::paint looks like "take this coordinate, add this nubmer to it, and call a method of QPainter". Drawing logic is outside, it must be written every time you want to draw a line. New API provides a way to just invoke DrawingPrimitive::draw giving it a QPainter and be done with drawing.
Looks good |
I'm sorry I had to turn it into draft, it feels like it went in a wrong direction. I'll try to find a way to make it better |
Closed in favour of #723 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi!
This is yet another bit of refactoring. The core idea is to create a common interface for such primitives like
qucs::Line
,qucs::Arc
,qucs::Area
and to hide drawing logic behind the interface.Look at this, where client has to select a proper method of
QPainter
and deal with its arguments not its own:— next time when rect is drawn everything has to be done again, and most likely will be just copied and pasted, making duplicate.
And compare with this:
— where
drawRect(cx + pa->x, cy + pa->y, pa->w, pa->h)
) is hidden and defined only once in member function bodyTo achieve this I did the following steps:
Area
and client had to decide somehow what this area is — an ellipse or a rectanglequcs::DrawingPrimitive
forqucs::Line
,qucs::Arc
,qucs::Rect
,qucs::Ellips
Everything should look the same, I haven't found any issues using the changed build on my machine.