16

Imagine the following two classes of a chess game:

TChessBoard = class
private
  FBoard : array [1..8, 1..8] of TChessPiece;
...
end;

TChessPiece = class abstract
public
   procedure GetMoveTargets (BoardPos : TPoint; Board : TChessBoard; MoveTargetList : TList <TPoint>);
...
end;

I want the two classes to be defined in two separate units ChessBoard.pas and ChessPiece.pas.

How can I avoid the circular unit reference that I run into here (each unit is needed in the other unit's interface section)?

9 Answers 9

24
+50

Delphi units are not "fundamentally broken". The way they work facilitates the phenomenal speed of the compiler and promotes clean class designs.

Being able to spread classes over units in the way that Prims/.NET allows is the approach that is arguably fundamentally broken as it promotes chaotic organisation of classes by allowing the developer to ignore the need to properly design their framework, promoting the imposition of arbitrary code structure rules such as "one class per unit", which has no technical or organisation merit as a universal dictum.

In this case, I immediately noticed an idiosynchracy in the class design arising from this circular reference dilemma.

That is, why would a piece ever have any need to reference a board?

If a piece is taken from a board, such a reference then makes no sense, or perhaps the valid "MoveTargets" for a removed piece are only those valid for that piece as a "starting position" in a new game? But I don't think this makes sense as anything other than a arbitrary justification for a case that demands that GetMoveTargets support invocation with a NIL board reference.

The particular placement of an individual piece at any given time is a property of an individual game of chess, and equally the VALID moves that may be POSSIBLE for any given piece are dependent upon the placement of OTHER pieces in the game.

TChessPiece.GetMoveTargets does not need knowledge of the current game state. This is the responsibility of a TChessGame. And a TChessPiece does not need a reference to a game or to a board to determine the valid move targets from a given current position. The board constraints (8 ranks and files) are domain constants, not properties of a given board instance.

So, a TChessGame is required that encapsulates the knowledge that combines awareness of a board, the pieces and - crucially - the rules, but the board and the pieces do not need knowledge of each other OR of the game.

It may seem tempting to put the rules pertaining to different pieces in the class for the piece type itself, but this is a mistake imho, since many of the rules are based on interactions with other pieces and in some cases with specific piece types. Such "big picture" behaviours require a degree of over-sight (read: overview) of the total game state that is not appropriate in a specific piece class.

e.g. a TChessPawn may determine that a valid move target is one or two squares forward or one square diagonally forward if either of those diagonal squares are occupied. However, if the movement of the pawn exposes the King to a CHECK situation, then the pawn is not movable at all.

I would approach this by simply allowing the pawn class to indicate all POSSIBLE move targets - 1 or 2 squares forward and both diagonally forward squares. The TChessGame then determines which of these is valid by reference to occupancy of those move targets and game state. 2 squares forward is only possible if the pawn is on it's home rank, forward squares being occupied BLOCK a move = invalid target, UNoccupied diagonal squares FACILITATE a move, and if any otherwise valid move exposes the King, then that move is also invalid.

Again, the temptation might be to put the generally applicable rules in the base TChessPiece class (e.g. does a given move expose the King?), but applying that rule requires awareness of the overall game state - i.e. placement of other pieces - so it more properly belongs as a generalised behaviour of the TChessGame class, imho

In addition to move targets, pieces also need to indicate CaptureTargets, which in the case of most pieces is the same, but in some cases quite different - pawn being a good example. But again, which - if any - of all potential captures is effective for any given move is - imho - an assessment of the game rules, not the behaviour of a piece or class of pieces.

As is the case in 99% of such situations (ime - ymmv) the dilemma is perhaps better solved by changing the class design to better represent the problem being modelled, not finding a way to shoehorn the class design into an arbitrary file organisation.

12
  • 1
    The compiler DOES enforce one class per unit when dealing with forms. Thus you have to jump through hoops to allow two forms to cooperate. Often this means it should be one form but sometimes you want independently movable pieces. Aug 17, 2009 at 4:09
  • 2
    @Loren: The compiler does not enforce any such thing. The VCL property streaming mechanism, in collusion with the linker and the form designer, forces one FORM class per unit, but you can introduce additional, non-form classes in a form unit just as you would any other unit. Independently moveable "pieces" of a form should arguably be components anyway, and if you want a visual design surface to create those components on, you can use frames to create those pieces. But yes, only one "piece" per frame (i.e form) unit.
    – Deltics
    Aug 17, 2009 at 9:34
  • 11
    The compilation speed argument made sense until 15 years ago. Today, I never have problems with compilation speed in any language. Only Delphi developers seem to be willing to give up flexibility to gain a few milliseconds per compile. From what I mostly see in the real world, this limitation does't promote clean OO design at all. It just forces people to make giant .pas files with 2934938 classes that can all see each other, and can even access each others private members. Aug 19, 2009 at 18:01
  • 2
    You're correct. Delphi's unit files are not "broken" but your assertion that "one class per unit/file" has no technical or organizational merit is flawed. Its merit lies in the speed of comprehension. As developers we spend 80%-90% of our time reading and understanding source code and only 10%-20% modifying it. Sifting through 30 marginally related classes in a file to modify only 1 of them is a time waster. Delphi's libraries were designed with multiple classes per file but the compiler certainly doesn't force this on anyone. As @Wouter points out it only encourages tight coupling. Aug 31, 2010 at 14:13
  • 2
    I wasn't implying that 1 class per unit should be adhered to blindly. Only that it has very real and significant benefits which you were overlooking. In addition to readability it also reduces the chance of merge conflicts and the need to recompile when a class is changed. Also most compiler messages report only file name and line number. With 1 class per unit its immediately obvious which class has an error before you even open the file. Yes, there are always exceptions to the rules. I just think 1 class per unit is the rule rather than the exception. Sep 1, 2010 at 20:38
16

One solution could be the introduction of a third unit which contains interface declarations (IBoard and IPiece).

Then the interface sections of the two units with class declarations can refer to the other class by its interface:

TChessBoard = class(TInterfacedObject, IBoard)
private
  FBoard : array [1..8, 1..8] of IPiece;
...
end;

and

TChessPiece = class abstract(TInterfacedObject, IPiece)
public
   procedure GetMoveTargets (BoardPos: TPoint; const Board: IBoard; 
     MoveTargetList: TList <TPoint>);
...
end;

(The const modifier in GetMoveTargets avoids unnecessary reference counting)

1
  • @user928177263 maybe. but once learned, the solution can be applied to larger variations of the same problem
    – mjn
    Oct 22, 2017 at 16:13
11

Change the unit that defines TChessPiece to look like the following:

TYPE
  tBaseChessBoard = class;

  TChessPiece = class
    procedure GetMoveTargets (BoardPos : TPoint; Board : TBaseChessBoard; ...    
  ...
  end;    

then modify the unit that defines TChessBoard to look like the following:

USES
  unit_containing_tBaseChessboard;

TYPE
  TChessBoard = class(tBaseChessBoard)
  private
    FBoard : array [1..8, 1..8] of TChessPiece;
  ...
  end;  

This allows you to pass concrete instances to the chess piece without having to worry about a circular reference. Since the board uses the Tchesspieces in its private, it really doesn't have to exist prior to the Tchesspiece declaration, just as place holder. Any state variables which the tChessPiece must know about of course should be placed in the tBaseChessBoard, where they will be available to both.

3
  • Accepted this, since it is short and provides a solution to my question. I voted all the other good answers up. Aug 24, 2009 at 7:38
  • I am getting an "E2086 Type 'tBaseChessBoard' is not yet completely defined" error.
    – IceCold
    Oct 22, 2017 at 10:52
  • @Alaun, this was not a fully working snippit, just enough to show the path to a solution. The tBaseChessBoard should have methods which are needed by GetMoveTargets but implemented as abstract or virtual.
    – skamradt
    Nov 29, 2017 at 17:28
3

It would be better to move ChessPiece class into ChessBoard unit.
If for some reason you can't, try to put one uses clause to the implementation part in one unit and leave the other one in the interface part.

4
  • Concerning your second point: as I mentioned, this does not work because I need the definition from the other unit in the interface section! Aug 16, 2009 at 18:31
  • 1
    Oh, I missed that :-) Is really necessary ChessPiece to know ChessBoard, or the other way around? Aug 16, 2009 at 18:38
  • Well, the chess piece should decide where it can move (there will be subclasses for the different pieces) and it needs to know the board to determine where it can go. The other direction is pretty clear. Aug 16, 2009 at 18:41
  • 1
    Maybe you should have TChessController for moving the pieces?
    – Harriv
    Aug 16, 2009 at 21:54
1

With Delphi Prism you can spread your namespaces over separate files, so there you would be able to solve it in a clean way.

The way units work are just fundamentally broken with their current Delphi implementation. Just look at how "db.pas" needed to have TField, TDataset, TParam, etc in one monstrous .pas file because their interfaces reference each other.

Anyway, you can always move code to a separate file and include them with {$include ChessBoard_impl.inc} for example. That way you can split stuff over files and have separate versions via your vcs. However, it's just a bit unhandy to edit files that way.

The best long-term solution would be to urge embarcadero to ditch some of the ideas that made sense in 1970 when pascal was born, but that are not much more than a pain in the ass for developers nowadays. A one-pass compiler is one of those.

3
  • 1
    Bummer that people just mass-downvote without commenting on which part they don't agree with. Aug 20, 2009 at 18:10
  • 2
    +1 for suggesting the one pass compiler should be history. I guess that the average increase of compile time will be no more than 5% for a two-pass compiler. Maybe somebody knows the exact figures :) ?
    – mjn
    Aug 20, 2009 at 19:29
  • 1
    Unit files aren't "broken". The only thing preventing those huge .pas files from being separated is backwards compatibility. Much of that coupling could be avoided by liberal use of interfaces. I agree about the one-pass compiler though. It places unnecessary requirements upon the developer and prevents some very useful run time optimizations which require multiple passes to accomplish. Sep 1, 2010 at 21:09
0

It does not look like TChessBoard.FBoard needs to be an array of TChessPiece, it can as well be of TObject and be downcasted in ChessPiece.pas.

3
  • -1 No, it can't. There are methods in TChessBoard that call i.e. FBoards [I, J].GetMoveTargets (...). In addition to that a downcast is not the clean solution I was looking for. Aug 16, 2009 at 19:58
  • then why didn't you say so in your question? I was just giving you one simple hint, no need to downvote me because I didn't know about your classes.
    – Ozan
    Aug 16, 2009 at 20:06
  • That's not the reason I downvoted. The reason I downvoted is that I don't consider a solution with a downcast a clean solution. But since I didn't explicitly ask for a clean object-oriented solution, I take my downvote back. Sorry about that. Aug 16, 2009 at 20:12
0

Another approach:

Make your board of tBaseChessPiece. It's abstract but contains the definitions you need to refer to.

The internal workings are in tChessPiece which descends from tBaseChessPiece.

I do agree that Delphi's handling of things that refer to each other is bad--about the worst feature of the language. I've long called for forward declarations that work across units. The compiler would have the information it needs, it wouldn't break the one-pass nature that makes it so fast.

0

what about this approach:

chess board unit:

TBaseChessPiece = class 

public

   procedure GetMoveTargets (BoardPos : TPoint; Board : TChessBoard; MoveTargetList : TList <TPoint>); virtual; abstract;

...

TChessBoard = class
private
  FBoard : array [1..8, 1..8] of TChessPiece;

  procedure InitializePiecesWithDesiredClass;
...

pieces unit:

TYourPiece = class TBaseChessPiece

public 

   procedure GetMoveTargets (BoardPos : TPoint; Board : TChessBoard; MoveTargetList : TList <TPoint>);override;

...

In this aproach chess board unit will include the reference of pieces unit only in implementation section(due to the method that will in fact create the objects) and pieces unit will have a reference to chess board unit in interface. If I'm not wrong this handle your problem in a easy way...

1
  • yep, that's pretty much the same solution as already proposed only with classes exchanged. BTW: why don't you use indents to make the code more readable? Just use 4 spaces indentation Aug 18, 2009 at 18:54
0

Derive TChessBoard from TObject

TChessBoard = class(TObject)

then you can declare procedure GetMoveTargets (BoardPos : TPoint; Board : TObject; MoveTargetList : TList );

when you call the proc, use SELF as the Board object (if you are calling it from there), then you can reference it with

(Board as TChessBoard). and access the properties etc from that.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.