Upcoming event

Be-Delphi Delphi Developer Day

Be-Delphi is organizing their first (of many) Delphi Developer Day on November 17th in Edegem near Antwerp. That day will be completely dedicated to Delphi and Prism.

At Be-Delphi, Devia will be holding a talk about the new LiveBindings in Delphi XE2, so be sure to grab a hold of me and say hello !

Simplify your Delphi Code using some OO techniques (Part 2)

written by Stefaan Lesage on 17/03/2010

In the First part of this series, we gave a brief overview of some general rules. Meanwhile we've had some time to think about what our code is supposed to do, so it's a good idea to continue with the second part.

Introduction

Well, we did get some time to think a little more about what our code is supposed to do, so we have a pretty good idea about it already. Sadly the people who originally created the code didn't really think about it. In this article we'll take a close look at how everything was programmed and we'll have a look at some of the disadvantages and how we should solve them.

The Old Code

Basically the developers though about it and they had the idea that settings should be read from the registry when the user wanted to edit them. Once the user finished changing the settings the code should store the new settings in the registry. Apparently they also had the idea to load the settings when the first form gets displayed and save them when it gets closed.

So, in the main form they had some code which looks like this :

procedure TfrmMain.FormShow(Sender: TObject);
var
  aRegistry : TRegistry;
begin
  aRegistry := TRegistry.Create;
  try
    aRegistry.OpenKey( '<THE_REGISTRY_PATH>', True);
    // FString, FInteger and FBoolean are private field declarations on
    // TfrmMain.
    FString  := aRegistry.ReadString( '<STRING_SETTING_KEY'  );
    FInteger := aRegistry.ReadInteger( '<STRING_SETTING_KEY'  );
    FBoolean := aRegistry.ReadBool( '<STRING_SETTING_KEY'  );
  finally
    aRegistry.Free;
  end;
end;

procedure TfrmMain.FormClose(Sender: TObject; var Action: TCloseAction);
var
  aRegistry : TRegistry;
begin
  aRegistry := TRegistry.Create;
  try
    // FString, FInteger and FBoolean are private field declarations on
    // TfrmMain.
    aRegistry.OpenKey( '<THE_REGISTRY_PATH>', True);
    aRegistry.WriteString( '<STRING_SETTING_KEY', FString  );
    aRegistry.WriteInteger( '<STRING_SETTING_KEY', FInteger  );
    aRegistry.WriteBool( '<STRING_SETTING_KEY', FBoolean  );
  finally
    aRegistry.Free;
  end;
end;

procedure TfrmMain.mnuEditSettingsClick(Sender: TObject);
var
  aEditSettingsForm : TfrmEditSettings;
begin
  aEditSettingsForm := TfrmEditSettings.Create( Self );
  try
    aEditSettingsForm.TheString := FString;
    aEditSettingsForm.TheInteger := FInteger;
    aEditSettingsForm.TheBoolean := FBoolean;

    if ( aEditSettingsForm.ShowModal = mrOK ) then
    begin
      FString  := aEditSettingsForm.TheString;
      FInteger := aEditSettingsForm.TheInteger;
      FBoolean := aEditSettingsForm.TheBoolean;
    end;
  finally
    FreeAndNil( aEditSettingsFrom )
  end;
end;

Will this work ?

As you can see, the code will be working. It will read the settings when the main form gets shown and store it in some private variables. When the main form gets closed, the settings will be stored. And there is even some code which will open a Edit Settings form, pass some values to it, and get some results back when the user closes the Edit Settings form by pressing the OK Button.

So what's the problem ?

Is the code executed at the right time ?

In my opinion the OnShow and OnClose events on the form arn't really the best places to execute this code. What if the settings included things related to the visual appearance of the form ? Do you want to read and change those each time this form gets shown ? Maybe you have some other settings which should get read even before the first form is shown ?

In any case you might want to read the settings or store them at different points during the execution of your application. In this example, that would require you to execute the OnShow event of the main form to load the settings. Even worst is having to rewrite or copy / paste the loading code in other locations. Believe it or not, but that's what's happening in a lot of cases.

What happens when a problem occurs while loading / saving the settingss ?

The code isn't perfect, and strange things will happen when a certain registry key doesn't exist. The code will raise an exception causing it to read or save only part of the settings and ending up with invalid settings. In our case, it would be nice if a default value was used when a certain registry key doesn't exist.

Additionally when the user opens the form to change the settings, and closes it using the OK button, the new settings are passed back to the main form, but are not stored in the registry yet. If some other part of your application now reads the settings from the registry again you might end up with different settings than the ones on your main form.

Is it really the responsibility of the main form to Load or Save settings ?

Well, in short NO ! Not at all !

What if we had to create a command line version of our application ? We might still need to read some settings from the registry, but we won't need any forms at all. What if I need to start a thread for some background processing, and need to read the settings in that thread as well ?

As you can see the Main From really shouldn't be responsible for the Loading and Saving of the settings. It can call a method to do the job, but the actual code should never be in the main form.

Another rule of mine is that a form should only contain code which has an influence on the visual aspect of the form or its components / controls. Reading settings and Writing settings doesn't have a visual inflence on the form, so it's code shouldn't be in a form. You might need to change some visual aspects based on what you read from the registry, but the actual reading itself isn't the responsibility of a form.

Is there a better alternative ?

Well, it's one thing to say that something is incorrect, but when doing so you should at least try to tell how to do it correctly. So, lets think a bit more about what we want and what we need. Generally speaking we want something to load and save application settings. It would be nice if we had some code we could use in whatever application we create in the future. It really doesn't matter how many settings we need to read, or what they are called, and what format they are the only thing we care about is that we can Load / Save / Edit and Use them. How this is done isn't our responsability but something the ApplicationSettings class should take care about.

If we need to use the settings in our applications we only need to use code which will look like this :

var
  aApplicationSettings : TApplicationSettings;
begin
  aApplicationSettings := TApplicationSettings.Create( '<OUR Registry Key>' );  
  try
    // Code used to load our settings  
    aApplicationSettings.LoadFromRegistry;
    
    // Code used to edit our settings
    if ( aApplicationSettings.EditSettings ) then    
    begin
      // Our settings got changed so we might want to do
      // something if necessary.
    end;
        
    // Code used to save our settings
    aApplicationSettings.SaveToRegistry;    
  finally
    FreeAndNil( aApplicationSettings );  
  end;
end;

Who's responsible for what ?

As you can see, our application / form or whatever doesn't really need to know how it should read the settings, it shouldn't really need to know how it is saving the settings, nor how we will display or edit the settings. The only thing it has to know is that we have Application Settings and we can use them.

Our Application Settings class on the other hand needs to know which settings the application needs, what kind of settings they are and how it should load / save each of those settings.

Are there any advantages to this approach ?

Absolutely, otherwise I wouldn't have taken the time to write all this down. Our main goal here is to use Delphi as an Object Oriented programming language, not just a RAD tool. We will be creating some classes based on our new approach, and our goal is to reuse those classes whenever necessary.

If we need to write another application tomorrow and it needs to save / load some application settings we will only have to let the system know which settings are needed, how they are called and what data type they are. All the rest of the code will simply be reused. We will spend a little more time creating the base classes and their code, but next time we need it, everything will be (almost) ready for us.

What's next ?

Well, we've been thinking and talking a lot so far, but we havn't really done any true coding yet. Maybe a few people are thinking they could have written the code in the time they read these two articles.

I'm quite sure that's possible, and I've seen it done ...

Sadly I've also been the guy who got hired 5 years after the application got written to debug the whole application in search of why something didn't work in some cases. I've also been the person who had to search through thousands of lines of code wondering why one setting never got saved or got replaced by an incorrect value. And yes, I've been that person who wished they had taken the time to think about their code before they actually started working on it :-P

Anyway, next time I'll show you how I would do it, and hopefully share some code with you.

Comments

  • 1

    It’s always good to be reminded of the basics of OO, it can be easy to be pushed into the first thing that works with mundane tasks like configuration!

    written by Jamie on 20/03/2010
  • 2

    Hi Jamie,

    I’m glad you found it interesting.  I’m quite sure there might be quite a few ways to solve simple things like this though.  The goal of this series of articles is to show how it could be done, which doens’t mean it should or it has to be done this way !

    Regards,


    Stefaan

    written by Stefaan Lesage on 21/03/2010
  • 3

    Add OnShow := NIL; as first line in the OnShow event, if any other application is messing around with your window handle it will break all logic, meaning that if your form’s settings changed in private variables and another application is messing with your form’s visibility for some reason, it will RELOAD your settings.

    written by delphigeist on 21/03/2010
  • 4

    Hi,Stefaan!

    Cool articles! You can find both (1st and 2nd) translation here:
    http://keeper89.blogspot.com/2010/03/delphi-1.html
    http://keeper89.blogspot.com/2010/03/delphi-2.html

    written by Yuri on 21/03/2010
  • 5

    Hi Delphigeist,

    Actually that was my whole point.  Reading settings is something which doesn’t belong in the .OnShow of you form since that can be executed quite a few times.

    It’s much better to override the constructor and put the Settings stuff in there.


    Regards,


    Stefaan

    written by Stefaan Lesage on 22/03/2010
  • 6

    Yuri,

    Glad you like the articles and nice to see you took the time to translate them.


    Regards,


    Stefaan

    written by Stefaan Lesage on 22/03/2010
  • 7

    Stefan, nice peace of work. But your talking about implementing a class instead of using the Onshow() and onClose() event. Where do you put the class in the form?

    written by John Kuiper on 23/03/2010
  • 8

    Hi John,

    I am indeed talking about creating my own class, and that’s the whole point of an Object Oriented programming language.  It isn’t the responsibility of my form to read / write settings, so from my point of view that logic / code should be separated from the form.

    In the course of these articles you’ll see the advantages of my approach.  Once everything is ready I’ll be able to reuse everything in another application or even in other parts of my code.

    In the next few articles I’ll try to describe how I went through the whole proces and how I ended up coding it.  I’ll try to explain the advantages and problems I encoutnered, so keep reading :-)

    Regards,


    Stefaan

    written by Stefaan Lesage on 23/03/2010
  • 9

    Stefan,

    What is the advantage if you’re using the class code once instead of putting it in the events of the form?

    written by John Kuiper on 23/03/2010
  • 10

    Hi John,

    Well, you’ll discover that in the next few articles.  But the most important thing is Code Reuse and putting your code where it belongs.

    Lets take the example of a bank.  If I want to transfer funds from my account to yours, I don’t need to know how it works, it just needs to work.  For the bank itself, it’s a completely different thing, it’s their responsibility and they will have to do some checks, rollback in case of problems, ...

    So, most people would have code which looks like this :

    —Check if account A is valid
    —Check if account B is valid
    —Check if account A has the necessary amount available
    —Take money from account A
    —Deposit it on account B
    —In case of errors go back

    But the whole process isn’t our responsibility, in fact we should only have code which looks like this :

    —TransferMoney( aFromAccount, aToAccount, aAmount);

    I don’t care how they handle it, that isn’t my job / responsibility.  The only thing I care about is that the job gets done, and gets rolled back if necessary, but how they do that ...

    It improves your code, the readability of your code and the maintainability of your code.  But as I mentioned, that’s something we will discuss in future articles.

    Regards,

    Stefaan

    written by Stefaan Lesage on 23/03/2010
  • Commenting is not available in this weblog entry.

    Archive