[Firebird-devel] Software Engineering 101

[Jim S. <ja...@ne...>]

When I sat down to create Interbase (InterBase?) in 1984, I used a block 
discipline that I developed writing PDP-11 Datatrieve, and used in just 
about every thing since.  Blocks, precursors to objects, shared a common 
block header that looked like this:

typedef struct blk {
    UCHAR       blk_type;
    UCHAR       blk_pool_id;
    USHORT      blk_length;
} *BLK;

A typical block started like this:

typedef struct dsql_rel {           /* Relation block */
    struct blk        rel_header;
    struct dsql_rel     *rel_next;       /* Next relation in database */
    ....
    }

To create a block, a routine called the block allocator passing the 
block type, pool, and optional length of repeating tail.  The allocator 
created and returned a block of the right size, with the header fields 
set to the appropriate values and the rest of the block cleared.

In Firebird/Vulcan, SQL has been promoted to full citizenship and now 
shares metadata with other engine citizens.  The DSQL code references to 
the former "dsql relation" object so frequently the conversion left a 
like-named object.  The new dsql_rel block is referenced through the 
internal relation block.  Depending on usage, an internal relation block 
may or may not have a dependent dsql_rel object.  If a dsql_rel object 
exists, the relation object's destructor releases it.  I assumed 
dropping the inheritance of "dsql_rel" from the pool allocator base 
classes and allocating it with the "new" operator would cut the links 
with the past.  In specific, dangling references to the block header in 
dsql_rel would announce themselves as compilation errors.

Last night I got an email from a Vulcan user reporting a block-type 
exception from a dsql_rel reference.  Couldn't possible be, I thought, 
there is no header to check.  But the user gave me the module and line 
number, which was indeed a DEV_BLKCHK macro for the offending object.  
Huh?  None of the compilers I use regularly are particularly bashful 
about giving errors when I reference non-existent fields.

On close examination, I discovered the following:

   * Yes, dsql_rel did inherit, indirectly, from blk.
   * Blk doesn't actually contain the block header information
   * The length, pool, and type information are hidden in an invisible 
prolog preceding an object
   * The DEV_BLKCHK macro passes the object to be checked as a void* to 
a method that grubs around the prolog.
   * The whole thing took a major dump on the floor when the inheritance 
structure changed.

This is a violation of just about every principle of software 
engineering.  It screwed up a perfectly, useful, reliable, maintainable 
block header mechanism.  Why did somebody change a working system to 
bury useful information, to wit the block type and memory pool? Why 
replace a simple, direct mechanism with one in which block type and pool 
can be accessed only with a type unsafe mechanism?

Is it possible that our resident C++ gurus, while expert in NUMA, RTTI, 
RAII, TLS, and garbage collection futures are unacquainted with the C++ 
concept of inheritance?

Guys, it works like this.  Declare a block header.  Back in the old days 
of short names and  hunt-and-peck typing I called it blk, but Block 
would be better.   Put the common block fields you want in tBlock.  
Declare the classes that need a header like this:

class MyClass : public blk

This will cause "MyClass" to "inherit" (as we say) the block header 
fields.  Even though they were declared in "Block", they are also 
accessible in "MyClass" as public members.  A private memory allocator 
could initialize these fields.  The "object oriented"  way  to 
initialize the data in Block would be to define a "constructor" on 
Block.  The class "Block" controls the fields it owns. This is called 
"modularity".  The memory allocator doesn't have to be in bed with the 
application. The code still works if a different memory allocator is 
used, say, the unadorned ("standard") "new".

There's a second major benefit. If "MyClass" is changed so it no longer 
inherits from "Block" references to the members of "Block" will produce 
compile errors, rather than just screwing up the system at some unknown 
future time like your miserable design just did to me.


Jim Starkey
Netfrastructure, Inc.
978 526-1376