[Lazarus] Circular references and code quality

[Martin]

On 04/10/2011 11:01, Juha Manninen wrote:
> Hi
>
> There is lots of duplicate code and data definitions between Lazarus 
> TProject and TLazPackage (and their ancestors).
> I tried to improve the situation by creating a common base class for 
> them. It is not possible because it ends up with a circular reference 
> hell.
>
> Almost 2 years ago I suggested to solve the problem in language syntax 
> somehow.
> http://lists.freepascal.org/lists/fpc-devel/2010-January/018909.html
>
> The comments always tell to use interfaces and abstract base classes, 
> and claim that this feature enforces good design.
> Bollocks!
> This feature enforces BAD design and implementation. Lazarus itself is 
> an example of it.
> Is copy-pasting code between Project and Package a sign of good 
> design. IMO it is not.
> Is the class structure somehow flawed if such problems happen. No it 
> is not. Such things happen in big, real projects.
>
> For the same reason there are many type-casts in Lazarus core units. A 
> variable is type-casted from base class to the real class.
introduce getter functions, or properties, and centralize the typecast. 
(see below)

> Another common solution is to copy all code into one big file. It is 
> not only a bad thing but it has some limits. You wouldn't want to copy 
> most Lazarus code into one file for example.
> Thus I found it strange that even Lazarus developers claimed they 
> never encountered such problems. Yet, they have clearly worked hard to 
> avoid circular references.
> It is almost like in a religious sect, nobody dares to admit it was a 
> mistake to join it.
>

IMHO it's a fine line.

I did encounter both, situation where I was forced to think about the 
design and ended up with a different and *better* design, because of 
this limit.
But also situation, where the opposite happened.

And while it may very well be, that in those cases where it was not an 
improvement, a better design might actually exist, the path to it 
appears blocked by tons of old code, grown over many years. Sure easy to 
say "refactor" (an I would love too), but resources are limited. In real 
live the amount of refactor one can do is limited, code grows over 
years, and not all of it is or will be as it should.

Anyway, having to accept the situation as it is, a few thinks I found 
helpful:

1)
Recent work on MainIDEIntf, which for some reasons is spread over at 
least 3 units. IIRC:
- MainIdeIntf => abstract base
- MainIDEBase => some implementations => this unit also has the global 
variable
- MainIDE => final implementation => this unit initializes the global 
var in MainIdeBase

Only I needed the variable in MainIdeIntf, as some units, which needed 
it, were themself already used by MainIdeBase
So I changed the variable in MainIdeBase to be a property (property 
outside of class declaration). That way it was easy to move it to 
another class, yet all code could still access it as wanted (and the 
property does the type cast once and for all. (Only needed to fix a few 
FreeAndNil as the property doesn't do for var param)

2)
The other problem I find, is that plenty of code requires circles, 
because some classes get to know about stuff they shouldn't.
I did experience that in SourceEditor => for some reaon it knew about 
the fpdoc build-in editor. IMHO it should not, and I replaced all that 
with events, that fpdoc editor can hook.

This 2nd points shows exactly the problem: a lot of code that has grown, 
and has created dependencies in the wrong direction. But it is an almost 
indefinte amount of work to change all that.

But this 2nd point is also the example that helped improving design. By 
reducing the amount of knowledge that some classes now have about 
everything else, while they shouldn't have it..