Overview
Examples
Screenshots
Comparisons
Applications
Download
Documentation
Tutorials
Bazaar
Status & Roadmap
FAQ
Authors & License
Forums
Funding Ultimate++
Search on this site
Search in forums












SourceForge.net Logo
Home » U++ Library support » U++ Widgets - General questions or Mixed problems » Overriding Display methods too complicated due to high amount of arguments (Making Display class easier to use)
Overriding Display methods too complicated due to high amount of arguments [message #55204] Sun, 18 October 2020 00:01 Go to next message
Klugier is currently offline  Klugier
Messages: 745
Registered: September 2012
Location: Poland, Kraków
Contributor
Hello,

I would like to tell you what I don't like about Display class (CtrlLib). It's two virtual methods are complicated to override due to the need to explicitly repeat the parameter list. It is exactly 6 parameters. Let's take a look at what we have now:
class Display {
public:
...
virtual void PaintBackground(Draw& w, const Rect& r, const Value& q,
	                     Color ink, Color paper, dword style) const;
virtual void Paint(Draw& w, const Rect& r, const Value& q,
		   Color ink, Color paper, dword style) const;


The solution for that is to introduce simply DisplayPaintContext that will contain all parameters variables:
class Display {
public:
...
virtual void PaintBackground(const DisplayPaintContext& ctx) const;
virtual void Paint(const DisplayPaintContext& ctx) const;

// Or
virtual void PaintBackground(Draw& w, const DisplayPaintContext& ctx) const;
virtual void Paint(Draw& w, const DisplayPaintContext& ctx) const;


Of course, it can be implemented in alternative way for example by providing first Draw parameter and rest as a context. Of course we can not simply change the declaration of this method. To fix the problem we should introduce the new one and deprecate the old.

Sample replacment in Display and Array examplex:
struct FontFaceDisplay : Display {
	virtual void Paint(Draw& w, const DisplayPaintContext& ctx) const override {
                auto r = ctx.GetRect();

                Font fnt = Font(ctx.GetValue(), r.Height() - 2);
                String txt = Font::GetFaceName(ctx.GetValue());
                w.DrawRect(r, ctx.Paper());
                w.DrawText(r.left + 2, r.top + (r.Height() - GetTextSize(txt, fnt).cy) / 2, txt, fnt, ctx.GetInk());
	}
};

struct MyDisplay : public Display {
    virtual void Paint(Draw& w, const DisplayPaintContext& ctx) const override
        w.DrawRect(ctx.GetRect(), paper);
        w.DrawEllipse(ctx.GetRect(), ctx.GetValue());
    }

};


Reference:
- Coding Revolution - Long Parameter List
- Refactoring Guru - Long Parameter List
- SonarSource - Static analyzer - Functions should not have too many parameters (4 is maximum limit)

Klugier


Ultimate++ - one framework to rule them all.

[Updated on: Sun, 18 October 2020 01:03]

Report message to a moderator

Re: Overriding Display methods too complicated due to high amount of arguments [message #55205 is a reply to message #55204] Sun, 18 October 2020 19:16 Go to previous messageGo to next message
Oblivion is currently offline  Oblivion
Messages: 758
Registered: August 2007
Contributor
As a user, my opinion:

I personally don't find Display paint methods confusing at all.

If the existing methods are going to be deprecated, however, I would suggest passing the reference of Draw and Value in DisplayDrawContext:

struct DisplayDrawContext
{
    Draw&        draw;
    const Value& value;
    Rect         rect;
    Color        ink   = SColorText;
    Color        paper = SColorPaper;
    dword        style = 0;
    DisplayDrawContext(Drww& w, const Value& q, const Rect& r) : draw(w), value(q), rect(r) {}
};

MyDisplay().Paint(DisplayDrawContext(w, q, r));



Copying the value is not always a good idea as it can contain large stuff.


Best regards,
Oblivion


[Updated on: Sun, 18 October 2020 19:35]

Report message to a moderator

Re: Overriding Display methods too complicated due to high amount of arguments [message #55214 is a reply to message #55205] Mon, 19 October 2020 15:59 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 12837
Registered: November 2005
Ultimate Member
Oblivion wrote on Sun, 18 October 2020 19:16


Copying the value is not always a good idea as it can contain large stuff.


Incorrect, does not depend on how large the stuff is.

You would have to try hard to create Value that is expensive to copy because it is large. In fact, I think it is impossible.

If it fits within 12 bytes, you copy just 16 bytes. If it does not, it is created once and then reference counted. Now reference counting is certainly a bit expensive (but not prohibitively), but definitely does not depend on how large the stuff is... Smile

Mirek

Re: Overriding Display methods too complicated due to high amount of arguments [message #55215 is a reply to message #55214] Mon, 19 October 2020 16:53 Go to previous message
Oblivion is currently offline  Oblivion
Messages: 758
Registered: August 2007
Contributor
Quote:
Incorrect, does not depend on how large the stuff is.

You would have to try hard to create Value that is expensive to copy because it is large. In fact, I think it is impossible.

If it fits within 12 bytes, you copy just 16 bytes. If it does not, it is created once and then reference counted. Now reference counting is certainly a bit expensive (but not prohibitively), but definitely does not depend on how large the stuff is...


Well now, thanks for the correction, this is excellent news.

And now I've looked into the Value API doc, it is indeed pointed out there. Embarassed Laughing


Best regards,
Oblivion


[Updated on: Mon, 19 October 2020 16:53]

Report message to a moderator

Previous Topic: How to use multi-tab control?
Next Topic: Add compilable testcases for nontrivial problems!
Goto Forum:
  


Current Time: Wed Oct 28 07:20:04 CET 2020

Total time taken to generate the page: 0.00897 seconds