Should I separate common units to Public and Internal, or have all Public?

[Mike Torrettinni 199]

I have a feature that spans across a few frames and few units. And I have CommonProjectDefinitions.pas where all types and common methods are defined.

One of the common types is main data type: TProjectDataType = (dtItems, dtInvoices, dtUsers, dtDocuments, ...);

Now I have a MainForm that ads a few Frames as Project data views and each has it's own TProjectDataType.

If I want for MainForm to know what datatype each Frame is, I need to use CommonProjectDefinitions.pas in MainForm,

 

Because only the TProjectDataType is used by MainForm (and eventually probably some other units), is this way OK (to expose the whole common unit) or it makes more sense to have CommonProjectDefinitions_Public.pas where all common types that are also used by other forms are defined - so I end up with CommonProjectDefinitions_Public.pas and CommonProjectDefinitions_Internal.pas (which contains everything that doesn't need to be Public).

 

The main concern is to have neatly enclosed units and not everything exposed.

 

Any suggestions or experience what to do in such case?

 

 

 

Edited May 21, 2020 by Mike Torrettinni

[David Heffernan 2454]

This sounds like a whole layer of extra complexity for no gain.

 

Frankly I'm not surprised that you seem to find everything so challenging when you choose to introduce layer on layer of complexity.

[Attila Kovacs 675]

"And I have CommonProjectDefinitions.pas where all types and common methods are defined."

 

You either stick with this "very oldschool" method or you explode this unit separated by the modules using it, but splitting it after public/bird/dog/whatever is definitely a way to nowhere.

Edited May 21, 2020 by Attila Kovacs

[Mike Torrettinni 199]

1 hour ago, Attila Kovacs said:

you explode this unit separated by the modules using it

for example: If MainForm has 2 frames, Project list and User list - same class/frame, different TProjectDataType... and different actions refresh different frame, something like this:

if aAction = actRefreshProjects then
  for i := 1 to n do
	if fFrames[i].ProjectDatatype = dtProjList then 
      fFrames[i].Refresh;
 
if aAction = actRefreshUsers then
  for i := 1 to n do
	if fFrames[i].ProjectDatatype = dtUsers then 
      fFrames[i].Refresh;

In this case MainForm needs to know about TProjectDataType - so it has to use a unit where it is defined.

 

How can I hide TProjectDataType from MainForm?

 

 

Edited May 21, 2020 by Mike Torrettinni

[Guest]

1 hour ago, Mike Torrettinni said:

How can I hide TProjectDataType from MainForm?

May be if you used different approach, like send aAction to fFrames, let each frame decide if it does need refresh or not, in other word decentralize those actions, like using plugins, if it is a refresh then send a refresh to all.

[Mike Torrettinni 199]

4 minutes ago, Kas Ob. said:

send aAction to fFrames, let each frame decide if it does need refresh or not,

OK, this makes sense. Let me try. thanks!

[Pawel Piotrowski 18]

Maybe have a look at the DEB an Event Bus framework for Delphi from Daniele Spinetti.
I think you will like it. It should help you achieve your decoupling without too much added complexity.

here is the blog entry about this:
http://www.danielespinetti.it/2016/02/deb-event-bus-framework-for-delphi.html

and here the git repo:
https://github.com/spinettaro/delphi-event-bus

 

[Lars Fosdal 1866]

Daniele's site seems to be blocked by the Syntax Highlighter from AlexGorbatchev.com being unreachable.

[Mike Torrettinni 199]

1 hour ago, Pawel Piotrowski said:

Maybe have a look at the DEB an Event Bus framework for Delphi from Daniele Spinetti.
I think you will like it. It should help you achieve your decoupling without too much added complexity.

here is the blog entry about this:
http://www.danielespinetti.it/2016/02/deb-event-bus-framework-for-delphi.html

and here the git repo:
https://github.com/spinettaro/delphi-event-bus

 

Thanks, will have a look!

[David Schwartz 442]

While your approach makes a little sense, many of us learned very painfully that centralized collections of things become a huge, ugly mess over time.

 

If you're using OOP, the notion of "encapsulation" carries over to other things besides classes. Classes are contained in things, and those things are contained in things, and so forth.

 

Put stuff in the unit that manages it. Use some form of dependency injection to pass objects into others that need them, and Factories to request things you need from common locations.

[Dmitriy M 10]

It seems like a simple factory class could solve your issue. Let it have bunch of class methods like CreateItemsFrame, CreateUsersFrame etc. Let it know about your TProjectDataType enums that will get passed to corresponding factory methods. Main form only needs to know about this factory class. If there is some actions that needs to be taken based on what type of frame is displayed on main form, then let the frames to expose some event that main form can subscribe to and trigger this event when required.

Edited May 23, 2020 by Dmitriy M

[Mike Torrettinni 199]

Thanks guys! This was great exercise, a lot of pen and paper to draw out how it should all come together. I have 14 frames, a few Callback functions between them, MainForm is cleaner and doesn't know about frames, so less units exposed.

 

On 5/21/2020 at 9:45 AM, Kas Ob. said:

let each frame decide if it does need refresh or not

Worked out very well! I even figured how to 'detect' which frame has been refreshed, to focus on correct Tab in PageControl, simple boolean return.

 

On 5/22/2020 at 9:01 PM, David Schwartz said:

Use some form of dependency injection to pass objects into others that need them,

This was one of the hardest things, to design classes that are independent, but still have connected hierarchy. I will probably need a few more exercise like this to fully use all the power of classes.

 

On 5/22/2020 at 10:05 PM, Dmitriy M said:

simple factory class could solve your issue

I think that is what I have, I just didn't follow those tutorials about Factories, because reading those examples is like reading about Interfaces... all alien to me, at this moment  🙂

 

[Guest]

@Mike Torrettinni As rule of thumb, use only what you are comfortable with, factories and interfaces have their merits, and yet have their disadvantage as they hinder you coding speed and might restrain and compicate your ability to update and extend ( may be even add features), but that is the point from using them in the first place, so without going in discussing when and where use any algorithm and why, simply put use the shorter and simpler version always, a version of code that you feel comfortable with.

 

Now to another suggestion and smaller one than DEB or other approachs, add new class or data module lets call it connector, in this connector you can call from mainform events, frames will register them selves to this connector which will maintain a list per event to be called, they will register for receiving each event they require, with this very simple code your main form will call this connector with many diffrent events like refresh, save, load, resize .. ,

resize for me is a must !, when mainform resize i want the frames to adjust too, VCL not aways adjust/reposition as i want, sometimes i want to hide few elements from frame to make it to more revlevant to the user who like the mainform smaller,

anyway frames who had refreshed and need to notify the mainform can call the connector and the connector will call the mainform or you can stick to your approach with return value, here you can add priority (integer instead of boolean) from each frame and the connector will call the mainform for any adjustment for one frame or more.

Such code has zero dependency between mainform and frames, and any changes introduced to one them will not require changing the other, not even look at its code.