tags:

views:

247

answers:

2

Just recently, probably because I've been maintaining some old code, I've started to look at how / why I do things. As you do.

Most of my Delphi programming has been picked up in house, or from examples scattered across the web or manuals. And in some things are done just because "that's how I do it"

What I'm currently wondering about is Declaration, of variables, procedures, functions, etc.

When I am working with a form, I will place all my procedures and functions under public or private. Whilst I will try to avoid global vars and constants will generally go under var or const, either in the interface or implementation, depending on where they need to be called (occasionally though they will go in public / private)

Otherwise, if its just a unit I will declare the procedure in the interface and use in the implementation. Some of the code I've been maintaining recently has no interface declaration but instead has everything properly ordered with calls after procedures...

Is there a correct way to do this? Are there rules of what should / should not go in the class? Or is it a style / when you started thing?

Edit to add

My question is not about whether a declaration of a procedure goes in private/public but whether all declarations in a TForm Unit should go in one of these. Similarly should var / const be in one or the other?

Further clarification

I understand that not declaring in interface, or declaring in public/private/etc affects the visibility of procedures/functions to other units in my applicaiton.
The core of my question is why would i not want to declare? - especially when working in a form/unit when placing in private is much more explicit that the thing declared is not available to other units...

Cheers Dan

+5  A: 

Everything that can have a different value depending on the concrete instance belongs to the class, i.e.

TDog = class
strict private
  FColor : TColor;
  FName : String;
public
  property Color : TColor read FColor write FColor;
  property Name : String read FName write FName;
end;

Color and name are clearly attributes of each dog (and each dog will have other values here).

General rules:

  • Fields belong in private (visible in this class and in this unit) or strict private (visible only in this class)
  • If you need access to fields from other classes, create a public property. This gives you the freedom to change the simple field access to a more sophisticated getter / setter method lateron without changing the interface of your class.
  • Everything should be as local as possible. If private is enough, there's no need to make it protected (visible in subclasses too). And only make those things public that you really need from the outside.
  • Forms: only those things that you want to be stored in the DFM file should be published.
  • Put as much as you can in the implementation section and as little as you can in the interface section. This is also true for uses clauses.

You might be confusing the term global variable. If it's declared in a class it's not a global variable (even if declared public). Global variables (which you correctly consider good to avoid) always go in a var section either in the interface or the implementation section (which is preferrable following the general rules above)

Smasher
One other rule of thumb: Be careful not to hide too much stuff away in the private section without exposing it via properties or other methods. Someone might want to inherit from your class later and need to use those members, and getting at them can involve some ugly hacks. See http://tech.turbu-rpg.com/79/abusing-extended-rtti-for-fun-and-profit for an example.
Mason Wheeler
agreed. `private` vs. `protected` deserves some deeper thaught.
Smasher
When writing and maintaining a library for the use of third-party developers, it is without doubt better to hide and not expose than to expose for others to access via inheritance. Designing for inheritance is hard, and the more you expose, the more coupling descendants will have. When in doubt, keep it hidden. That maximizes flexibility for future maintenance without breaking clients.
Barry Kelly
I mentioned potentially "contentious" design questions in my answer; and _inheritance_ is almost certainly something that would feature. In my experience, inheritance is generally abused as a means of reuse. Large and deep inheritance hierarchies actually limit re-usability and induce other elements of poor design. Don't get me wrong; it's a powerful technique, but: "with great power comes great responsibility".
Craig Young
+3  A: 

The question seems to deal with scope. In other words, how easily accessible things can or should be.

As a general guideline, you want to reduce the scope of things as much as possible but still keep them accessible enough to be reused. The reason for this is:

  • that as your system grows and becomes more complex, the things that have are larger scope are more easily accessible.
  • as a result, they are more likely to be reused in an uncontrolled fashion.
  • (sounds great) but the problem comes when you want to make changes, and many things use that which you want to change...
  • it becomes far more difficult to make your changes without breaking something else.

Having said that, there is also a distinction between data (variables, constants, class fields, record attributes) and routines (functions, procedures, methods on classes). You'll want to apply the guidelines far more strictly to data because 'strange use' of data could interfere with some of your routines in highly unexpected and hard to debug ways.

Another thing to bear in mind is the special distinction between global variables and class fields or record attributes:

  • using global variables there is only one 'value' (term used loosely) for the entire application.
  • using class fields or record attributes, each new instance of the class or record has its own values independent of other instances.
  • This does seem to imply that you could use some form of global whenever your application only needs one thing. However, as alluded to earlier: this is not the only reason to avoid globals.
    • Personally I even tend to avoid global routines.
    • I'm frequently discovering that things that seemed okay declared global are not as universal as first thought. (E.g. Delphi VCL declares a global Screen object, I work on 2 screens; and many of our clients use 4 to 6.)
    • I also find it useful to associate routines that could have been global with specific classes as class methods. It generally makes it easier to understand the code.

So listing these 'locations' from largest scope to smallest, you would generally strive to pick locations lower down in the list (especially for data).

  • interface global
  • implementation global
  • interface threadvar
  • implementation threadvar
  • published (Note I don't really consider this to be a scope identifier; it's really public scope; "and includes RTTI information" - theoretically, it would be useful to also mark some private attributes as "include RTTI".)
  • public
  • protected
  • private
  • strict private
  • local variable

I must confess: what I have presented here is most certainly an over-simplification. It is one thing to know the goals, another to actually implement them. There is a balancing act between encapsulation (hiding things away) and exposing controlled interfaces to achieve high levels of re-usability.

The techniques to successfully balance these needs fall into a category of far more complicated (and sometimes even contentious) questions on system design. A poor design is likely to induce one to expose 'too much' with 'too large' a scope, and (perhaps paradoxically) also reduce re-usability.

Craig Young
I think the argument against a `Screen` singleton isn't sound. You work on two monitors and not two screens, and there is an array of monitor objects for you to work with. OTOH there are other things in `TScreen` that are indeed singletons, like the active form or control, or the lists of cursors and fonts.
mghie
Yes, I'm familiar with the `Monitors` array; but it seems very much like a patched on afterthought to deal with the problem I mentioned. Note that `TScreen.DesktopHeight`/`Left`/`Top`/`Width` did exist (and probably still does) along-side the `Monitors` array. Also, I stated the example more as a means of illustrating how easy it is to make poor assumptions about what are suitable as singletons/globals. In fact, some software designers advocate: one should **never** design for Singleton. Imagine the possibilities if instead of `SysUtils.CompareStr` we had `TStringUtils.CompareStr(); virtual;`
Craig Young