Q; moving class helper code to a separate unit .. any disadvantages?

[d7_2_laz]

Not quite sure if it is right by concept ... so, maybe somebody knows better:

moved a longer class helper code (approx. 300 lines) to a separate unit
so that it - encapsulated - might be easier to use for other contexts too later.
The code is heavily used (let’s say for an advanced drawing of core elements of a lcl component).

Are there any "don't do's", caveats especially regarding performance loss (frequent calls across separate units)?

[jamie]

All the other helpers live in remove units while you use them, so I don't think it matters.

I know in D, you can't extend the helpers if that is what you did? but it's possible D has changed that, I would need to look.

[d7_2_laz]

"remove units " = remote units? If yes, i'd understand it would be ok as a usual practice without any impact on performance. Is this right?

"D" = Delphi?

" extend the helpers". Hm, it's about that i needed (unfortunately)  to clone a whole painting routine for to have a slight modification (skip of one step, otherwise i couldn't achieve a desired behaviour).
The code is long as it serves a couple of properties and they all should be preserved.
That's already done times ago as part of a subclassed component (no helper here involved).
Purpose described here (click onto the gif at the bottom for to play it):
https://forum.lazarus.freepascal.org/index.php/topic,60259.msg450449.html#msg450449

Now i wanted to have this code as part of a "TOpenTreeView = class helper for TTreeView" in a separate unit, so that i can apply it to any treeview (ie. picked up from the IDE) easily without only a very few footprints in the using program itself.
Works fine, but i'm a somehow curious if my approach is a bit crazy.

[TRon]

...
Works fine, but i'm a somehow curious if my approach is a bit crazy.

fwiw since helpers were added I use the same approach to extend existing classes and be able to test the addition. If it is generic functionality that benefits the class then I use/copy the same (helper) code to use in my inherited or custom class. If the helper is too specific then I usually do not bother and keep it a helper so that it can at least be re-used without too much work in another project.

So, in case it is considered crazy then I'm just as mad as a hatter like you seem to be 

[Thaddy]

@d7_2_laz
You are not supposed to do that, but anyway we have in trunk {$modeswitch multihelpers} which may help you with your sins.

[d7_2_laz]

Thanks for the feedbacks, very appreciated!

TRon, höm, nice symbol … maybe, sometimes i'm simply inspired to achieve something (here: something that ie. Win Explorer or Thunderbird do since ages) ... and then i try to find a solution ... whatever etc.

Thaddy, but ... why "not supposed to do that"?? (Otherwise i'd need to request an additional control variable, and such is surely not advisable for a particular purpose).
Thanks for pointing out a $modeswitch multihelpers.  Will dig a bit to find what's about.

[TRon]

Thanks for pointing out a $modeswitch multihelpers.  Will dig a bit to find what's about.

Before tat switch existed you were required to inherited the helper from another (previous defined/used/wanted) helper in case you wanted to add multiple helpers. With this modeswitch you do not have to inherit anymore, which makes defining helpers a lot easier because you do not have to think about if there is already another helper active (that you want to use at the same time).

Because that switch did not exist before, it inspired me to use separate units for helpers of a specific class/record/type because that allows for multiple helpers to be active and be present in the same unit (so that you wont have to look through all your code-base to be able to locate that one helper that you did not actually wanted to use and was "activated" by accident). I kept the structural approach but sometimes split up different helpers in their separate unit so that it is possible to switch a specific helper in/out more easily (and thus do not need to have all helpers active at the same time).

[d7_2_laz]

Thank you for those explanations, that’s very informative!
Just actually I’d have no need for such coexistence of helpers, but as we all know (I guess) such may change quickly.
Actually I’d only need one unit (let’s call it uTreeviewDrawHelper) to which a longer code is emigrated having only a few changes of the standard drawing (from TCustomTreeView.DoPaintNode)
and which is activated by one single procedure call for:

Code: Pascal  [Select][+][-]

1.   TOpenTreeView = class helper for TTreeView
2.    public
3.      procedure TreeViewH_AdvCustDrawItem_Hovered(Sender: TCustomTreeView;
4.            Node: TTreeNode; State: TCustomDrawState; Stage: TCustomDrawStage; var PaintImages, DefaultDraw: Boolean;
5.            bPreserveLastHoveredIdxMSStyle, bIsHotInStateFix: Boolean);

Tomorrow (so I understand so far) with $modeswitch multihelpers another app  could use, coexistencely with this unit, another unit “B” adding, via TOpenTreeView, additional/other functionality. That’s good to know. I already was wondering momentarily, what’s about now if I’d need another helper.
 

In the “helper” I see the possiblity not to e dependent on that the code is part of a full coded component.

Any vetos against the approach?

[PascalDragon]

Are there any "don't do's", caveats especially regarding performance loss (frequent calls across separate units)?

Such calls are simply direct calls. There is no difference between them being in the same unit or in different units.

In addition to that even inlining would work in principle.

[d7_2_laz]

Procedure inline .... interesting point PascalDragon, thanks!
In a first quick test it appears to work nicely, and i didn't see any negative effect here so far.

Might spend an additional support, ie. when thinking of rapid paintings of nodes following mouse moves or wheel scrolls.

[d7_2_laz]

Just besides as an info:
During my tests (and i never saw such before:) i had sporadically (and in an for me not quite reproducable order) some compile errors: "Error: Compilation raised exception internally". Or: "Fatal: no memory left" (whereas heaptrc reports: “no unfreed blocks").  Or, an error was reported within Generics.Collections.
That happened mostly since testing with TypeHelper in a separate unit ..... and, here, escpecially when inlining and here when having changed the signature of the inlined procedure for to test some simplification; or, when having switched between projects.
Ok. had recognized and mostly a clean recompile after deleting all contents of the "lib" subdir helped.

Occasionally, today i did read this and it appears to me it's probably related:
https://www.lazarusforum.de/viewtopic.php?f=5&t=7547&start=45
with refererence to this:
https://gitlab.com/freepascal.org/fpc/source/-/issues/25797
Interesting ... so be aware when testing changes here.