Skip to content

Coding Conventions

Michalis Kamburelis edited this page Feb 17, 2019 · 9 revisions

Table of Contents

Pascal Coding Conventions

In general, we follow the standard Lazarus and Delphi coding conventions, used throughout most modern Object Pascal code.

These are documented nicely on:

In particular:

  • Indent by 2 spaces. No tabs.

  • Use CamelCase for everything.

    • Including constants. So write MyConstant instead of e.g. MY_CONSTANT.

    • Including local variables. Even 1-letter variable names (so write I instead of i).

    • Including type names. Even the type names that are Pascal keywords (so write String / Boolean instead of string / boolean). Note: this rule was changed during CGE 6.5 development. So you will find a lot of code using lowercase string now in engine sources, but new code should use String.

  • Put "begin" on a separate line. I.e. do not mimic C "K & R" style (https://en.wikipedia.org/wiki/Indent_style#K.26R) in Pascal:

    // DON'T WRITE THIS:
    for I := 1 to 10 do begin
      Writeln(I);
    end;
    

    Instead, the "begin" should usually be indented the same as "end".

    // THIS IS OK:
    for I := 1 to 10 do
    begin
      Writeln(I);
    end;
    

    To look simpler, it's OK to omit begin/end when they would surround only 1 statement:

    // THIS IS EVEN BETTER:
    for I := 1 to 10 do
      Writeln(I);
    
  • The "else" keyword is written on a new line, unless it's right after "end". So:

    // THIS IS OK:
    if Foo then
      Bar
    else
      Xyz;
    
    // THIS IS ALSO OK:
    if Foo then
    begin
      Bar
    end else
    begin
      Xyz;
    end;
    
    // THIS IS ALSO OK:
    if Foo then
    begin
      Bar
    end else
      Xyz;
    
    // THIS IS ACCEPTABLE, BUT BETTER AVOID IT:
    if Foo then
      Bar
    else
    begin
      Xyz;
    end;
    
    // THIS IS NOT OK:
    if Foo then
    begin
      Bar
    end
    else
    begin
      Xyz;
    end;
    
    // THIS IS NOT OK, BUT IS USED IN A LOT OF CODE:
    // (Michalis was using this convention for a long time,
    // until it was pointed to him that it doesn't look optimal,
    // and Michalis agreed :)
    // Do not use this in new code, but don't be surprised if it still occurs somewhere.
    // Michalis will gradually get rid of it in CGE sources.)
    if Foo then
      Bar else
      Xyz;
    
  • Never leave trailing whitespace at the end of lines (in the long run, it causes unnecessary diffs when someone removes it).

  • Never use "with" keyword. Using "with" makes the code very difficult to read, as some of the symbols inside the "with A do begin .... end" clause are bound to A, and some are not, and it's completely invisible to the human reader which symbols are which.

    And it's impossible to determine it, without intimately knowing the complete API of class/record A.

    E.g. what does this code do?

    with A do
    begin
      SourceX := X;
      SourceY := Y;
    end;
    

    Does it modify A contents, or does it modify outside variables, merely reading the A contents? You really don't know, until I show you the documentation of the class of A, and all it's ancestors.

    Compare with a clear:

    SourceX := A.X;
    SourceY := A.Y;
    

    or

    A.SourceX := X;
    A.SourceY := Y;
    

    The "with" also makes the code very fragile to any changes of A API. Every time you add a new field/property/method to A, then the code inside "with A do begin .... end" may change it's meaning. It may compile, but suddenly will do something completely different.

    Likewise, every time you remove a field/property/method from A, the code inside "with A do begin .... end" may compile, if you happen to have a variable outside of this block with a name matching the name inside A.

  • The uses clause of our units and examples should follow the order

    • standard units (RTL, LCL, VCL...)
    • then our own (CastleXxx) units
    • then eventual game-specific units (GameXxx)

    Each part should start from a newline.

    // THIS IS OK:
    uses SysUtils, Classes,
      CastleUtils, CastleSceneManager,
      GameStatePlay;
    
  • Indenting inside classes:

    type
      TMyClass = class
      private
        MyField: Integer;
        procedure Foo;
      public
        MyPublicField: Integer;
        procedure Bar;
      end;
    

    If you use the nested types / constants, indent the fields inside the var block as well. See the example below, notice that MyField is now indented more than in the example above. It's not perfect -- MyField indentation is now inconsistent with MyPublicField. But on the other hand, MyField indentation is consistent with MyNestedConst and TMyNestedClass and how you usually indent var block.

    type
      TMyClass = class
      private
        type
          TMyNestedClass = class
          end;
        const
          MyNestedConst = 123;
        var
          MyField: Integer;
        procedure Foo;
      public
        MyPublicField: Integer;
        procedure Bar;
      end;
    
  • File extensions:

    • *.pas files are units,

    • *.inc are files to be included in other Pascal source files using $I (short for $Include).

    • *.dpr and *.lpr are main program files. We will soon rename all program files to *.dpr. While Lazarus accepts either .dpr or .lpr extension for the program file, Delphi tolerates only .dpr extension. So, like it or not, we have to adjust to Delphi, and just use .dpr.

  • All the engine functions are "reentrant", which means that they are safe to be called recursively, even through your own callbacks. E.g. the TFileProc callback passed to EnumFiles can call EnumFiles inside it's own implementation.

  • Some naming conventions:

    • If some procedure modifies it's 1st parameter then I usually end it's name with "Var" ("to variable").

      Often you will be able to see the same operation coming in two flavours:

      function DoSomething(const X: SOME-TYPE, ...): SOME-TYPE;
      procedure DoSomethingVar(var X: SOME-TYPE,...);
      

      The 1st (functional-like) version is more flexible, but the 2nd version may be faster (especially if SOME-TYPE is large, or requires time-consuming initialization).

      See e.g. CastleVectors and CastleImages units.

      This rule doesn't apply when SOME-TYPE is some class instance. It's normal that a procedure may modify the given class instance contents, no need to signify this with a "Var" suffix.

    • The term "stride" refers to a distance in bytes between memory chunks, following OpenGL conventions.

      If somewhere I use parameters like V: ^SOME-TYPE and Stride: Integer then it means that these parameters define a table of SOME-TYPE values. Address of 1st item is V, address of i-th is (V + i * Stride).

      Stride may be negative. Stride may also be 0, then it means that Stride = SizeOf(SOME-TYPE).

  • Compilation symbols used:

    Standard FPC and Delphi ones: MSWINDOWS, UNIX, LINUX, CPUI386, CPUX86_64, FPC to differentiate between compiler versions, and some more.

    See castleconf.inc.

    We also use DEBUG symbol. The build tool when compiled in debug mode (--mode=debug) defines the DEBUG symbol, and adds some runtime checks, like range checking and overflow checking. You can use {$ifdef DEBUG} in your own code to add additional things. There's also the RELEASE symbol, but usually we don't check for it's existence -- if DEBUG then we're in debug mode, else we're in release mode.

  • Exceptions' messages:

    • Do not start them with 'Error: ' or anything else that just says "we have an error". This is redundant, since all exceptions signal some error.

    • Don't end the Message with '!' character. Do not cause panic :) The exception message must look normal when presented to end-user.

    • Usually, Message should be a single sentence, and not end with the '.' character. But we do not follow this rule 100%, it's OK to break it for good reasons.

    • Message should not contain any line-breaks. Reason: this doesn't look good when displayed in some situations. Especially when one Message is embedded as part of the Message of other exception.

      We do not follow this rule 100%, it's OK to break it with good reasons. We know that some information really looks much cleaner when split into multiple lines (e.g. TMatrix4.ToString output is multi-line already).

    • Message should not contain any general program information like ApplicationName, ExeName etc. (The exception to this rule is when such information is really related to the error that happened, may help to explain this error etc.) In normal situation, the code that finally catched and outputs this exception should show such information.

  • Callbacks: "of object" or not?

    ObjectPascal is a hybrid OOP language and it has global function pointers and method pointers. They are incompatible, since the method pointer is actually two pointers (the class instance, and the code address). When designing a function that takes a callback, you're faced with a problem: define "a pointer to a method" or "a pointer to a global function/procedure"?

    In the past, I often chose to use "a pointer to a global function/procedure". With a generic "Data: Pointer" parameter, to allow passing user data. This is easier to use when you don't have a class instance (and you don't want to create a dummy class just for this), and it's always allows to add overridden version with "of object" callback (passing object instance as the Data);

    Nowadays, I usually define "of object" callbacks, assuming that all non-trivial code is usually in some class, and the "of object" is more natural to be used in OOP.

  • Order of methods: place the implementation of constructors (Create*) first, then destructor (Destroy), and then the rest of methods. I do not have a precise rule about the ordering of the rest of methods -- I usually like to group related methods together.

Other languages

  • Indent by 4 spaces in Java and Objective-C.

  • Never use tabs. (Unless they are inherent to the language, like Makefile).

  • Follow the standard coding conventions for that language.

Optimization guidelines

If you want to suggest some optimization (of speed, of memory usage) to the engine, especially if it:

  • makes a significant code complication to the existing code,

  • or it adds a significant amount of new code (which is also a code complication)

... then always first do some tests/thinking whether it's really worth it.

There are many situations where optimizing is not a good idea, because it will not change the "bottleneck" code (which means that the speed / memory use of something else is so large (in comparison) that it completely "masks" the thing that you optimize, making it irrelevant). In such cases, optimization is actually harmful, because the code quality goes down --- the optimized code is usually longer and/or more convoluted.

(Exception: in the rare cases when the optimized code is also shorter and cleaner, you have a full green light to do it just because the code quality is better.)

Bottom line:

  • We want to have less code.

  • We want to have simpler code.

  • Do not optimize just because you have an idea how to make some line of code faster. This thinking often leads to performing many tiny optimizations (and thus reducing code quality) that have no noticeable effect on the execution speed or memory use of real applications. First test/think whether it's worthwhile to optimize this piece of code.

As you can see, I put more emphasis on thinking about code quality than optimization. That is because I see some of us often making the mistake of not caring about code quality enough, and instead rushing to make an optimization (that lowers code quality for little-to-no gain to the final applications).

Of course, this does not mean we don't want to optimize. It just means that we require justification for each optimization, the optimization must have a noticeable effect on some real-world use-case. We want the code to be fast and use little memory -- there are various ways to achieve this, often using smart algoriths on CPU, and/or thinking about how the CPU cache is used, and/or delivering data in better chunks to GPU. Low-level optimization of some local routine is not always the most effective approach.

There is also a dreaded "death by 1000 cuts" that we want to avoid, which is sometimes caused by missing a number of small optimizations that would have a noticeable effect overall. E.g. that's why we use "Single" throughout the engine code, not Double or Extended. (except some special code where we have testcases that "Single" precision is not enough). Using "Double" everywhere would have a noticeable negative effect on the speed (yes, I tested it long time ago). But e.g. paranoidally avoiding calling "Sqrt" in the engine... proved to be usually a useless optimization, causing various bugs and not achieving any speed gain.

So, there are cases to be made for some low-level optimizations. But don't fall into the trap of implementing lots of useless low-level optimizations blindly.

Submitting your code contributions

It's best to use GitHub's pull requests.

  1. Fork the https://github.com/castle-engine/castle-engine/ . This is done by clicking on the appropriate button on GitHub.

  2. Clone your fork (i.e. download it to your local computer).

  3. Optional: Create a new branch in your fork, just for this specific feature, e.g. doing git checkout -b my-new-feature. This allows to separate your work on various CGE features.

  4. Work on your feature, committing and pushing as usual, to your branch in your fork.

  5. When ready, submit a pull request using https://github.com/castle-engine/castle-engine/pulls

See GitHub documentation (and other sites) for information about pull requests:

Advantages of pull requests:

  • They allow you to comfortably work on your pull request, committing and pushing and showing your changes to anyone. There is no need to ask for any permission to do this. (But, if you want, you can of course let us know about your work, see https://castle-engine.io/talk.php . We may be able to advise on a best way to add something to CGE.)
  • They allow us to use "code review" features of GitHub. This is a comfortable way to comment on your changes.

  • They allow everyone to submit, review and merge the changes relatively easily. And all operations can be done using the command-line or web interface, so it's comfortable / easy / flexible for everyone.

If for some reason you really cannot follow this workflow, it is OK to simply send a traditional ".patch" file, done by "git diff" or "svn diff" (you can access https://github.com/castle-engine/castle-engine/ as a GIT or SVN repository.) You can attach it to a new issue.

Castle Game Engine:

Wiki Contents:

Build Tool:

Mobile devices:

Other:

Upgrading:

Clone this wiki locally
You can’t perform that action at this time.