From STUPID to SOLID Delphi code (part 1 of n)
Delphi

From STUPID to SOLID Delphi code (part 1 of n)

By 5 maart 2020 september 29th, 2020 No Comments

In my almnost 25 years of programming in Delphi, I have seen a lot of code. A lot of awesome code, a lot of #facepalm code and a lot of Legacy code which nobody wants to touch because ‘It is running … se we leave it alone’. And although I do respect the ‘legacy’ code you carry with you over the years, I hate it when I have to facepalm again when I see newly written routines or imporovements. So I decided to start a little series about Going from STUPID to SOLID delphi code, so here we go…

Before we begin

Ok … before we start … It’s not my intention to point fingers at anyone’s code, or try to show you that I’m holier than the pope … We’ve all been there in the past and we’ve probably all been scratching our heads when we were presented with code we wrote 20 years ago.

So in this series I might be showing some example code, some code I have written 20 years ago, or some code I have seen in projects I have worked on. The code is used for reference and just to show you some easy fixes or easy ways to make your code easier to understand and maintain. Personally I strongly believe in the fact that you can only learn from your mistakes once you know which mistakes you have made and what you should do to not make them anymore.

But before we jump into the geeky code stuff, lets spend this first installment on the theory. What do I mean when I’m talking about STUPID or SOLID Delphi code?

STUPID and SOLID Delphi Code … what is it all about?

Both STUPID and SOLID are just some acronyms used quite often when talking about source code. It’s nothing more than a simple set or rules and principles with regards to developing software. Remember … these are just principles, not laws on stone tablets. There are always exception to those laws, but if we try to stick as close as possible to those principles, we will all be writing better code.

STUPID Code, and what’s wrong with it.

We have all written STUPID Delphi code, yes … I’m guilty as well. But what is STUPID code and why isn’t it a good thing?

  • Singleton
  • Tight Coupling
  • Untestability
  • Premature Optimization
  • Idescriptive Naming
  • Duplication

That’s what the Acronym stands for. It might not ring a bell, but I will be trying to explain all of this with a few ezamples, and I hope at the end of this chapter you will all be nodding your head.

Singleton

The Singleton Pattern is probably something we’ve all been using, and probably abusing as well. The idea is that an Application needs one, and ONLY one instance of an Object. The applicaiton will use Lazy Initialization and global accessors. Good examples in delphi are the TApplication, TScreen or even the TClipboard classes. It seems logic that your application has only one TApplication instance for example.

If possible though, we should try to avoid Singletons, especially if you can replace it with something else. Why ? Well … it’s very difficult to test Singletons, so Unit Testing on them is quite hard. Most of the time you have no control over when it gets created or destroyed, which can make life difficult as well. Singletons are considered Controversial to some, and an anti pattern to others.

Tight Coupling

Tight Coupling, or Strong Coupling, is the fact that for example you can’t use Class A without including Class B, which means you have to add UnitA, UnitB and UnitC to your project. This in turn means you need Unit13, Unit17 and Unit23 from a common project. And that in turn …

Another aspect of Tight Coupling is the fact that changing something in one class or unit means you have to change 250 things in 35 other units as well. To me, when opening a project / unit / form / datamodule it quickly becomes obvious that we have way too Tight Coupling in our Delphi projects. In the next few posts in this series I will be trying to show you some examples of tight coupling and why it isn’t a good thing.

Untestability

Every bit of code we write should be testable. Sadly I have seen tons of code which is untestable and the main reason is the Tight Coupling.

When trying to create a Tes case for a specific feature it should be possible to isolate that feature as much as possible. In the perfect circumstances you should be able to make a new project and only add the files you need to test that features. In many cases I have seen, that is nearly impossible. Adding Unit A to a project means you have to add 35 other units due to dependencies.

Furhtermore in many cases all actual logic is coded in forms or frames (for me personally … this is already a showstopper). Heck … a unit test shouldn’t even include any visual things (most of the time), but in our case … we have to add the form to the unit test, or we have to duplicate the code! Adding the form to the unit test means we have to add all units associated with that form. In the end we have a project which includes 75 units of which only 2 we should actually need for the testcase.

Code / features / functionality should be testable at any time ! Unit tests are a great thing, but in many cases it’s too difficult to write Unit Tests. The reason it’s too difficult ? Well because we have STUPID Code with very tight coupling.

It should be clear now that Tight Coupling is a bad thing and should be refactored as much as possible.

Premature Optimization

Optimizing too soon in the development cycle isn’t a good idea. Optimization should be done at the end of a the cycle. I’ve seen people write very complex code to optimize the scrolling in a grid. Now … that optimization worked and scrolling was smoother, but the code was so complex that nobody could fix it. In the end, the complex code could be removed by adding a BeginUpdate and EndUpdate around the process, but nobody dared to touch the code.

The easier fix was actually to just not select 250.000 records from the database if you can only display 25. Once I fixed the SQL statement, the whole scrolling optimization Delphi code could be completely removed.

Indescriptive Naming

We’ve all seen this, we’ve all done this … drop an edit box on a component and write code like edit1l.text := ‘Some Value’. All fine and dandy when you’re working on that specific thing, but 1 year later you have no idea what Edit1 was. Same thing with variables or even values.

Let’s have a look at this piece of code :

if ( Container.Type = 1 ) then
  // Metal
else if ( Container.Type = 2 ) then
  // Cardboard
else if ( Container.Type = 3 ) then
  // Plastic
else if ( Container.Type = 99 ) then
  // Unknownd

We are lucky this time because the developpper added a comment letting us know what the numbers mean. But this shouldn’t be necessary ! In this case making some constants with descripive names, or even an Enumerated type will make the code more readable even without comments :

Type
  TContainerType = ( ctMetal = 1, ctCardboard, ctPlastic, ctUnknown = 99 );

procedure Test;
var
  aContainterType : TContainerType;
begin
  case aContainterType of
    ctMetal: ;
    ctCardboard: ;
    ctPlastic: ;
    ctUnknown: ;
  end;
end;

Readable code, even without comments. Any new developer will immediately know wich code is executed when the ContainerType is ctCarcboard.

These days with code completion I would even say … try not to abbreviate when naming things. It’s not because our computers can only understand 0 and 1 that we should be cryptic with our names. After all … programming languages are made for humans to read, and computers to executed.

Duplication

Duplicating code is bad ! It is very bad, and I can’t stress this enough. I know … copy / paste is quick and easy in the short run, but will cause errors, regression bugs, unreadable code and hard to test / maintain code in the long run. Need the same piece of code twice ? Refactor it into a function / procedure. Don’t Repeat Yourself (DRY) and Keep it simple, stupid are two rules. Yes … as a developer you can be lazy, but be lazy the right way and write code only once!

SOLID Delphi code, and why it’s better!

So, i’ve tried to explain the different aspects of STUPID code, and I hope by now some heads will be nodding. If you’re just like me you will think the code you wrote in the past is really STUPID, but no worries … as long as we improve ourselves and start writing SOLID Delphi code, everything is fine.

Just like STUPID, SOLID is just a collection of principles / rules which you can follow to write better code. Again … just some principles. Some might not be obvious in Delphi, but still … try to stick as close to the rules as possible and it will increase your skills and make you write SOLID delphi code.

But what does SOLID mean ?

  • Single Responsibility Principle
  • Open/Closed Principle
  • Liskov Substitution Principle
  • Interface Segregation Principle
  • Dependency Inversion Principle

Single Responsibility Principle

What is SRP ?

The Single Responsibility Principle means that every class should have a single responsibility. Additionally the rules state that Single Responsibility Principle means that there should never be more than one reason for a class to change.

Might be difficult to explain, but in short … just because you can add everything into a single class, doesn’t mean it’s a good idea. Thinking of ‘who is responsible for what’ will make sure you code follows the Single Responsibility Principle. This in turn will reduce the Tight Coupling as well.

Example of how not to handle responsibility

When adding new logic always think “is my class responsible for that?”. As an example … a form can have a Save button, which does a lot of things. First it validates some edit boxes, then it executes some stored procedures to check a few things, then again it checks some visual controls and does some additional checks when certain boxes contain certain values. Next it will put the contents of the edit boxes into some fields on a dataset. After that it will Post the changes, and maybe even execute some more stored procedures if we were adding a new record, but not when we were editing a record. In some cases, it will delete records, and maybe even add some history records to the DB.

I know … I’ve exagerated for this example, but lets go with it …

How to use the Single Responsibility Principle correctly

First time I opened that code my brain immediately went “… but … but … Why?”. So thinking back to the “Is my class responsible for XXX” rule :

  • Is the form responsible for validating edit boxes … YES … in some cases
  • Is the form responsible for Executing Stored Procedures … NO
  • Is the form responsible for Posting the changes … YES … but actually it should just send the contents of the edit boxes to the class responsable for storage.
  • Is the form responsible for knowing how the posting should be done … NO
  • Is the form responsible for deleting records if XXX … NO
  • Is the form responsible for adding history records … NO

So … how should this be done then ? Well … my Save should look like this :

procedure TForm_DelRef.acSaveExecute(Sender: TObject);
begin
  // Lets pretend our edit controls are DataAware controls so the values
  // entered in the edit boxes are put into the corresponding dataset fields
  // otherwise we would have to do this ourselves

  // Saving should trigger validation, and the actual save and raise an
  // exception if the Validation / Save fails.
  aDataModule.SaveChanges;
end;

In short … pressing the Save on the form should just put the contents of the form’s edit boxes into the correct fields, and send a message off to save the data. The save button doens’t have to know how the validation has to be done, nor what the Save actually does. It passes along the responsibility to another class. That class might split responsibility to yet another 5 classes if necessary.

Benefits of using SRP

The result will be less Tight Coupling in your code, less repeating code and that will lead to code which is easier to maintain. If something is wrong with the validation ? Fix it in the class responsible for that, and not in the 10 forms where you could save the changes.

Open/Closed Principle

What is the Open/Closed Principle

With the Open / Close Principle we are refering to the fact that Classes / Modules / Routines should be open for extension, but closed for modification. Again … a bit difficult to explain, but in short … it should be possible to extend / add features to a class without the need to change other classes / code which use the said class.

This might imply that you need to add another layer of objects to your model. If I have a set of classes which handle storing stuff in a database, then it should be possible to add the necessary code to save the same data to another type of database without the need to modify the classes we already have. The principle here is that you should have Abstract classes which have the necessary routines to do some stuff, but the actual way to do said stuff implemented in descendant classes.

Example of how to not handle the Open / Closed Principle

I’ve seen code where in a specific routine of a class there was code like if (Self is XXX) or (Self is YYY) to handle a different workflow depending on the class. This is bad … terribly bad … If there is a different implementation between Class XXX and Class YYY then this logic needs to be moved down the chain into the corresponding class.

Example of the Open / Closed Principle in the VCL.

A great example here is the TDataSet class. On it’s own it has the necessary methods to handle post, delete, insert,… but the actual implementation isn’t done on TDataSet but might be done at a lower level in the hierachy TCustomADODataSet (ADO), TIBCustomDataSet (Interbase), TCustomSQLDataSet (Unidirectional DataSet / dbExpress), TFDDataSet (FireDac). So in our case the TDataSet is Open for Extension but closed for modification.

Liskov Substitution Principle

Liskov Substitution Principle

Another silly sounding name to me, but in short the Liskov Substitution Principle means that any object / class in your coude should be replacable with instances of their subtypes / descendants without the need to modify the correctness of you application.

Example of code which isn’t conform the Liskov Substitution Principle

TAdnimal = class
  procedure Eat; virtual; abstract;
  procedure Fly; virtual; abstract;
end;

TDuck = class( TAnimal )
  procedure Eat; override;
  procedure Fly; override;
end;

TCat = class( TAnimal )
  procedure Eat; override;
  procedure Fly; override;
end;

...

procedure TCat.Fly;
begin
  raise EException.Create( 'Silly You ... a cat does not fly' );
end;

In our case the TCat breaks the LSP (Liskov Substitution Principle), because in your code … if you would replace instances of TAnimal with an instance of TCat the result might be different, unexpected or not work at all. I know … it’s the worst example ever … but I couldn’t come up with something better.

How to fix the LSP in our bad example?

TAdnimal = class
  procedure Eat; virtual; abstract;
end;

TFlyingAnimal = class( TAnimal )
  procedure Fly; virtual; abstract;
end;

TDuck = class( TFlyingAnimal )
  procedure Eat; override;
  procedure Fly; override;
end;

TCat = class( TAnimal )
  procedure Eat; override;
end;

It’s still a bad example, but at least we’ve fixed the LSP Problem 🙂

Interface Segregation Principle

What is the Interface Segregation Principle

The Interface Segregation Principle refers to the fact that you are better off having many smaller interfaces than one big fat one. Think of it as the Single Responsibility Principle for Interface. If you are implementing an interface on a class and have to add methods / properties from that interface to your class which have absolutely no meaning on that class then your Interface Segregation Principle has failed.

Example of code which isn’t conform the Interface Segregation Principle

IShape = Interface
  function Area;
  function Circumference;

  property Length;
  property Width;
  property Radius;
end;

TSquare = Class( TShape, IShape )

end;

TCircle = Class( TShape, IShape )

end;

In this case we would have to make sure the Radius property is available on TSquare and the Width and Length property is available on TCircle. In both cases, that’s tatally unnecessary.

How to make our code conform to the Interface Segregation Principle

IShape = Interface
  function Area;
  function Circumference;
end;

IRectangle = Interface( IShape )
  property Length;
  property Width;
end;

ICirecle = interface
  property Radius;
end;

TSquare = Class( TShape, IRectangle )

end;

TCircle = Class( TShape, ICircle)

end;

Again … not the best example, but it illustrates the point.

Dependency Inversion Principle

In short the Dependency Inversion Principle means that your high level modules should not depend on the low level ones. Both of them may depend on abstract classes, but not on eachother. Also … abstraction should not depend on details. For example … Business Logic should not depend on DB Logic, UI / Form logic should not dependon Database logic, and of course the other way around too.

Example of bad Dependency Inversion

Any code where your form needs access to the DB Classes in order to do something is an example. Your form should not depend on DB Components, nor should the DB Components depend on the form. That’s why we have a thing called a DataSource, it allows our forms to get data from a dataset without directly accessing the dataset itself.

Another example :

TMailer = Class( TObject )
end;

TSendWelcomeMessage = class( TObject );
  FMailer : TMailer;

  constructor Create( aMailer : TMailer ); 
end;

procedure TSendWelcomeMessage.Create( aMailer : TMailer );
begin
  Inherited Create;
  FMailer := TMailer;
end;

This code fails the DIP because TSendWelcomeMessage accepts an instance of a TMailer in it’s constructor. But this makes the constructor of TSendWelcomeMessage depend on a concrete and not an abstract thing.

Example of Dependency Inversion Principle

So … how could we fix the previous example ? Well we would have to extract the common methods up to an abstract level. It’s a good idea to use interfaces for that since those are just a contract and contain no implementation details.

IMailer = Interface
  procedure Send;
end;

TSMTPMailer = Class( TObject, IMailer )
  procedure Send;
end;

TPhysicalPostMailer = Class( TObject, IMailer )
  procedure Send;
end;

TSendWelcomeMessage = class( TObject );
  FaMailerInterface : IMailer;

  constructor Create( aMailerInterface : IMailer ); 
end;

procedure TSendWelcomeMessage.Create( aMailerInterface : IMailer );
begin
  Inherited Create;
  aMailerInterface := IMailer;
end;

So we have extracted the Send method to the IMailer interface. That is very abstract and actually what we want. The TSendWelcomeMessage constructor now gets fed the abstract interface, and it will still work.

Coonclusion and Finishing words

So … I’m done with the whole STUPID vs SOLID delphi code theory, and I hope after reading this some of you are nodding your heads in some places.

Wether you’ve been nodding your head or not … when writing / modifying our Delphi code think about the following set of rules / principles :

  • The only code on a form should be code which influences the visual components on the form. All other code should be calls to methods on other classes.
  • Big chunks of code should be reworked into smaller routines to avoid too tight coupling and make sure we only have the code in a class which is actually necessary on that class.
  • 2 or more dots in a single statement in our delphi code probably means your code is not responsible for that specific logic and it should be reworked.
  • (ab)use Datamodules … they can be (ab)used for more than just holding a TDataSet.
  • Tight Coupling should be avaided at all time!

I know … I know … 20 pages of text condensed into 5 tips 🙂 But it’s worth seeing some examples of what we mean with STUPID and SOLID Delphi code. But if you try to stick to those simple 5 rules you will quickly notice your code will improve, will be easier to maintain, extend, extract and test.

After a while … your brain will be starting to use the SOLID coding principles automatically. Writing SOLID delphi code isn’t hard … once you noticed how to improve your STUPID code. In any case … if you want to learn more, there is a lot of info on the internet, and I’m here to help you if need be. In future blogposts I will try to show some more examples of STUPID vs SOLID coding in Delphi, so stay tuned if you want to read more. And if you have any feedback … use the comment section, it’s here just for that.

Further Reading

If you think I’ve invented all of this myself, you overestimated me 🙂 I do read a lot on this topic so I’ve borrowed a few things here and there. If you really want to know more about writing SOLID Delphi code (or SOLID code in general), feel free to check the links in the text and the following posts :

Leave a Reply