Jump to content
Arnaud Bouchez

Clean Code and Fast Code Slides at EKON 22

Recommended Posts

Regarding the naming of units. It makes sense to name a unit like this: ProjectName.UnitName.pas. ProjectName can be the abbreviated name of your project. And if the project is really big (like Delphi) - add the name of the subproject instead, for ex.: System.Types, Vcl.Forms, FMX.Forms etc. An even Subproject.Subproject.UnitName.pas: FMX.Forms.Border.pas, FMX.ListView.Appearances.pas... 

Share this post


Link to post

Slides 74-76. Strongly-Typed Pascal !!! 

Someone in Embarcadero should hear programmers request and help to make the implicit explicit in such situations:

program TestMetersToMiles;
{$APPTYPE CONSOLE}
type
  TDistanceMeters = type Double;
  TDistanceMiles = type Double;

function ConvertMetersToMiles(Distance: TDistanceMeters): TDistanceMiles;
begin
  Result := TDistanceMiles(Double(Distance) / 100000 * 62.14);
end;

var
  meters: TDistanceMeters;
  miles: TDistanceMiles;
begin
  miles := 100; // No syntax error, no warning, no hint
  meters := ConvertMetersToMiles(miles); // No syntax error, no warning, no hint
end.

I've complained already about this here.

Edited by Kryvich

Share this post


Link to post

What happens if you put this in a local procedure, and call it instead ?

 

procedure Main_Function;
var
  meters: TDistanceMeters;
  miles: TDistanceMiles;
begin
  miles := 100; // No syntax error, no warning, no hint
  meters := ConvertMetersToMiles(miles); // No syntax error, no warning, no hint

end;

 

Share this post


Link to post

@Rollo62 Nothing will change. Delphi allows implicit conversions between different types spawned from one common type (String, Integer, Double etc.). 

 

In fact, I wrote about this even 10 years (or so) ago in the old Delphi.Non-Tech forum. Unfortunately, no new warnings or compiler hints have been added since.

 

Strong typing worth nothing while we can write it as

miles := meters * Pi + Application.MainForm.Width;
// Another example with a comparison
miles := TDistanceMiles(100);
meters := TDistanceMeters(100);
if miles = meters then
  ShowMessage('Hi, it is your compiler!');

without any warning or hint from the compiler.

Edited by Kryvich

Share this post


Link to post
2 hours ago, Kryvich said:

Regarding the naming of units. It makes sense to name a unit like this: ProjectName.UnitName.pas. ProjectName can be the abbreviated name of your project. And if the project is really big (like Delphi) - add the name of the subproject instead, for ex.: System.Types, Vcl.Forms, FMX.Forms etc. An even Subproject.Subproject.UnitName.pas: FMX.Forms.Border.pas, FMX.ListView.Appearances.pas... 

Yes, camel-casing or using commas are options.
Since some of the code is expected to run on Delphi 7 (for several convenient reasons, no technical ones) we didn't use comma.
But the point of the proposal was that we won't use the project name, but the *folder* name within the unit name, to ensure 1. that the file is easy to locate in the SCM or at file level 2. the folder logic is not about projects, but to ensure data/ui/projects/etc... are uncoupled. Folder layout, therefore unit name layout, therefore the system architecture, should not be project-oriented, but domain-oriented - see https://martinfowler.com/tags/application architecture.html

Edited by Arnaud Bouchez
  • Like 1

Share this post


Link to post
7 hours ago, Kryvich said:

Slides 74-76. Strongly-Typed Pascal !!! 

Someone in Embarcadero should hear programmers request and help to make the implicit explicit in such situations:


program TestMetersToMiles;
{$APPTYPE CONSOLE}
type
  TDistanceMeters = type Double;
  TDistanceMiles = type Double;

function ConvertMetersToMiles(Distance: TDistanceMeters): TDistanceMiles;
begin
  Result := TDistanceMiles(Double(Distance) / 100000 * 62.14);
end;

var
  meters: TDistanceMeters;
  miles: TDistanceMiles;
begin
  miles := 100; // No syntax error, no warning, no hint
  meters := ConvertMetersToMiles(miles); // No syntax error, no warning, no hint
end.

I've complained already about this here.

You'll need to declare two record types for this. The language isn't going to change now. 

Share this post


Link to post

@David Heffernan No need to change the language. We need a small revision of the compiler so that it shows a hint or warning.

Such checks can be added to a third-party tool similar to FixInsight. But having additional type checking right in the compiler would be much better.

Share this post


Link to post

 

@Kryvich Changing the compiler requirements on such would break backward compatibility. This is why I doubt it would happen.

 

Another problem is about the overloaded functions. The compiler will complain about ambiguous usage, and will abort compilation.
And another is about type helpers: it is for a given type, so you can't use double.ToString as TDistanceMeters.ToString IIRC. I complained against that when they introduced type helpers - I guess noone at EMB uses user defined simple types (whereas I use it everywhere).

 

@David Heffernan Defining records is indeed the proper way of ensuring very strong type checking. Especially with the implicit conversion introduced with Delphi 10.3. And you would gain methods, e.g. for making conversions explicit (which is even better).

But in practice, I like the fact that "type double" gives at least some information in the IDE (e.g. code completion) which is not needed to be put as parameter or function name.

Edited by Arnaud Bouchez

Share this post


Link to post

@Arnaud Bouchez If someone don't care about the strong typing, these hints and warnings can be mooted selectively, locally for a code block or globally for a project.

{$WARN STRONG_TYPE_CHECKING OFF}

They have already added improved type checking for string types when there is an implicit ANSI <--> Unicode conversion:

[dcc32 Warning] W1057 Implicit string cast from 'AnsiString' to 'string'

It is very useful in many cases. So I see no problem adding the same check for other simple types, the built in ones or created by the user.

 

As for type helpers, the compiler might allow their use for spawned user types if the strong type checking warning I suggest is disabled. 

Share this post


Link to post

Perhaps the other way around: let strong typing be OFF by default (backward compatible) but enable it to ON for new code requiring it. 🙂

About Unicode/AnsiString it was a requirement due to another breaking change.

  • Like 1

Share this post


Link to post

Exactly! Let this check be disabled by default.
Actually Unicode strings (WideString type) existed before converting VCL to Unicode. Try it in Delphi 2007 - no warnings, but there is conversion error:

procedure TestAnsiWideString;
var
  ansiStr: AnsiString;
  wideStr: WideString;
begin
  wideStr := #1055#1072'-'#1073#1077#1083#1072#1088#1091#1089#1082#1091#13#10
    + #12498#12517#12540#12473#12488#12531#12289#31169#12383#12385#12395#12399#21839#38988#12364#12354#12426#12414#12377#12290;
  ansiStr := wideStr;
  if wideStr <> ansiStr then
    Writeln('Houston, we have a problem.')
end;

But they added the Ansi <---> Unicode check, because it became a massive problem when moving to Unicode.

Share this post


Link to post
3 hours ago, Kryvich said:

@David Heffernan No need to change the language. We need a small revision of the compiler so that it shows a hint or warning.

Such checks can be added to a third-party tool similar to FixInsight. But having additional type checking right in the compiler would be much better.

Since the language regards these two types as assignment compatible, it's pretty hard to imagine adding warnings or hints when you write such assignments.

 

Wrap the value in distinct record types.

Share this post


Link to post

Yes, wrapping to the record type is possible. Though having the [optional] strong type checking for these types would be better. Detecting a possible error at compile time is much better than at runtime, right? The language already has everything to implement this:

type
  // Aliases, no type checking on assignment
  TDistanceMeters = Double;
  TDistanceMiles = Double;
  // New custom types, [optional] type checking on assignment is needed
  TDistanceMeters = type Double;
  TDistanceMiles = type Double;

 

Edited by Kryvich

Share this post


Link to post

It's interesting that for spawned class types there is a special type check:

type
  TApple = class end;
  TOrange = type TApple; // Identical inside but not the same semantically

procedure TestApplesAndOranges;
var
  apple: TApple;
  orange: TOrange;
begin
  apple := TApple.Create;
  orange := apple; // <-- [dcc32 Error] E2010 Incompatible types: 'TOrange' and 'TApple'
end;

Great!

Update: But:

  orange := TOrange.Create; // E2010 Incompatible types: 'TOrange' and 'TApple'
  orange := TApple.Create;  // E2010 Incompatible types: 'TOrange' and 'TApple'

Hmm. So what is the orange that I have declared? We can find out like this:

 

var
  orange2: TObject;
...
  orange2 := TOrange.Create;
  ShowMessage(orange2.ClassName); // 'TApple'

This is the correct answer (the classes are the same inside). Only it is not clear why orange := TOrange.Create; does not compile...

Edited by Kryvich

Share this post


Link to post
4 hours ago, Kryvich said:

Yes, wrapping to the record type is possible. Though having the [optional] strong type checking for these types would be better. Detecting a possible error at compile time is much better than at runtime, right? The language already has everything to implement this:


type
  // Aliases, no type checking on assignment
  TDistanceMeters = Double;
  TDistanceMiles = Double;
  // New custom types, [optional] type checking on assignment is needed
  TDistanceMeters = type Double;
  TDistanceMiles = type Double;

 

But the language was designed differently. Use records. What's so difficult about that? 

Share this post


Link to post

@David Heffernan Pascal was designed by Wirth with a Static  Strong  Safe typing in mind. The proposed compiler warning follows this concept. It can help prevent some semantic errors and write a clean code, without the need to complicate a code by introducing new structured types. So It's good for Delphi programmers and for Delphi, a win-win situation.

Share this post


Link to post
1 hour ago, Kryvich said:

@David Heffernan Pascal was designed by Wirth with a Static  Strong  Safe typing in mind. The proposed compiler warning follows this concept. It can help prevent some semantic errors and write a clean code, without the need to complicate a code by introducing new structured types. So It's good for Delphi programmers and for Delphi, a win-win situation.

It's fairly clear that the Delphi language we are referring to was designed to make such types assignment compatible. That is made clear by the documentation for the Delphi language.

 

There are other languages that have taken different approaches, but we aren't discussing those languages here. We are discussing Delphi. Whilst you might wish that a different decision had been taken, it wasn't. It's really not likely to change.

Share this post


Link to post

I am sure using the type keyword after the equals sign was not part of the original pascal syntax and is only there to let the compiler produce a new typeinfo instead of it just being an alias that still points to the exact same type.

Share this post


Link to post

Thanks for sharing ab, the slides about server-side architecture is really inspiring!

 

Speaking of 'clean code', I admire your ability to architect the entire mORMot's huge codebase which includes a lot of module, supports a lot versions of Delphi, supports a bunch of compilers on various platforms.

 

And I have a humble advise - do you think the the `with` statement makes reading and debugging harder? 

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×