A Question of Style

[rgdawson 8]

Nick Hodges excellent book, "Coding in Delphi", gave some interesting ideas about creating Sqlite classes and interfaces, then anonymous methods, then enumerators.  I am ever mindful of the idiom, "just because you can doesn't mean you should", but I decided to experiment.  Below are some examples and what I am asking is, what looks best to you?

 

I started with Yuri Plashenkov's simple Sqlite classes (https://github.com/plashenkov/SQLite3-Delphi-FPC)

 

I converted the classes into interfaces which gives me Pattern 1 below.  Then I added some functions that take advantage of anonymous methods and that gives me Pattern 2.  Then I added enumerators and that gives me Pattern 3.

 

{...}
var
  SQL: string;
  Stmt: ISQlite3Statement;
  Name: string;
  Age: integer;
begin
  SQL := 'SELECT Name,' +
         '       Age'+
         '  FROM People';

{Pattern 1: using interfaces...}
  Stmt := DB.Prepare(SQL);
  while Stmt.Step = SQLITE_ROW do
  begin
    Name := Stmt.ColumnText(0);
    Age := Stmt.ColumnInt(1);
    {...}
  end;
  
{Pattern 2: adding anonomous methods...}
  DB.FetchAll(SQL, procedure(Stmt: ISqlite3Statement)
    begin
      Name := Stmt.Column[0].AsText;
      Age := Stmt.Column[1].AsInt;
      {...}
    end);

{Pattern 3: adding enumerator...}
  for Stmt in DB.Prepare(SQL) do
  begin
    Name := Stmt.Column[0];  {this uses class operator Implicit to call .AsText}
    Age := Stmt.Column[1];   {this uses class operator Implicit to call .AsInt}
    {...}
  end;

I have also been considering the idea of style with respect to the Sqlite functions .ColumnText() and .ColumnInt() etc.  For example instead of .ColumnText(0), I could have .Column[0].AsText, or using "Implicit" , just .Column[0], .Column[1] etc where .AsText, .AsInt, etc is called implicitly.

 

Many of you folks on this forum are the most brilliant I am aware of, and I have learned a ton over the years reading your stuff here, on Stack Overflow, in your books, and in your code.  Someday, I'll retire and someone will have to take over this code and I want it to be straight forward looking, easy to read and understand. 

 

Which would you say is preferable?

 

R Greg Dawson
 

Edited March 25 by rgdawson
comma missing

[Steven Kamradt 20]

My personal preference is to decouple the database field logic completely.  Use a mapping object to map property name to field name and just deal with an object model and its properties. 

 

If I have to pick from only your examples, I would pick pattern 2.  I dislike pattern 3 the most from a future maintenance point of view.  It hides the context of the assignment and could make some logic bugs difficult to locate.  Pattern 1 doesn't give you the ability to add additional attributes to the column.  Pattern 2 allows you to extend the concept of a column record/class to have additional methods which might be useful that could be used in a loop. 

if not Stmt.Column[x].isNull then ....

if Stmt.Column[x].valueType = vtString then ...

ColumnHeading[x] := Stmt.Column[x].fieldname;

Yes, you can do the same with pattern 3, but that pattern of using implicits will make it harder to read intent later, especially if you are mixing types in the same block with the same syntax. 

 

[rgdawson 8]

19 hours ago, Steven Kamradt said:

My personal preference is to decouple the database field logic completely.  Use a mapping object to map property name to field name and just deal with an object model and its properties. 

 

If I have to pick from only your examples, I would pick pattern 2.  I dislike pattern 3 the most from a future maintenance point of view.  It hides the context of the assignment and could make some logic bugs difficult to locate.  Pattern 1 doesn't give you the ability to add additional attributes to the column.  Pattern 2 allows you to extend the concept of a column record/class to have additional methods which might be useful that could be used in a loop. 

if not Stmt.Column[x].isNull then ....

if Stmt.Column[x].valueType = vtString then ...

ColumnHeading[x] := Stmt.Column[x].fieldname;

Yes, you can do the same with pattern 3, but that pattern of using implicits will make it harder to read intent later, especially if you are mixing types in the same block with the same syntax. 

 

 

Nice, good points.  After implementing a block of code all different ways and then just staring at it for awhile, I also decided Pattern 2 seems best.  Sometimes, you don't really know until you try it with real code.  Using the anonymous method pattern for the fetch loop code makes it easier to just plainly see that database fetching is going on and just seems more elegant.  As for getting column/field data, pattern 3, using implicit, indeed, made the code harder to read and contextualize.  Then, your three little lines of code there are all good ideas, too, which I intend to implement.  Thanks.

[David Schwartz 426]

Einstein is credited with saying, "Everything should be made as simple as possible, but no simpler."

 

There are usually three steps in most non-trivial algorithms:

 

* declaration / initialization (can be optional)

* process

* destruction (may be automatic)

 

I like it when each of these is clearly identifiable. I get nervous when the class designer tried to be too cute and save me from typing a line or two of code, so I end up wasting time digging into the code to see what's being done implicitly. Eg., sometimes there's an implicit object being used that you need to copy stuff into before anything will work. Sometimes the result set is in a (hidden) object that you need to deal with, starting with whether it's even valid or not.

 

You can shuffle things around in many interesting ways to simplify how much code you need to write in different places, but just be sure it's clear and unambiguous so when you look at it in six months you can quickly figure out what it's doing. None of these three examples does a good job of that, IMHO. 

Unless you're passing in an already prepared query object, then you need to prepare it -- that the first item.

 

If it succeeds, then you want to process the query, typically iterating over what might well be 'n' result sets.

 

Finally, you want to free up the object(s), although this may be done automatically.

 

So for me, Pattern 1 fits this best with the separate Prepare statement, but the 'process' part can be improved.

 

Try this approach and see how you like it. Note that this doesn't conflict with what Nick teaches. Just don't try to do everything in one statement. 🙂

 

 

 

Edited April 9 by David Schwartz

[rgdawson 8]

Thanks for the comments.  After playing around with these approaches, I eventually decided I did not like patterns 2 and 3.  Pattern 1 seems it would be easiest to read and understand when seeing it for the first time or coming back to it six months later.  Yes, DB.Prepare creates the statement interface, which is reference counted so it will get finalized and disposed of when it goes out of scope.

 

So I ended up going with something like this.

 

Stmt := DB.Prepare(
  'SELECT Name' +
  '       Age,' +
  '  FROM People');
while Stmt.Step = SQLITE_ROW do
begin
  Name := Stmt.SqlColumn[0].AsText;
  Age := Stmt.SqlColumn[1].AsInt;
  {...}
end;

I also like doing something like

 

with DB.Prepare(
  'SELECT Name' +
  '       Age,' +
  '  FROM People') do Fetch(procedure
begin
  Name := SqlColumn[0].AsText;
  Age := SqlColumn[1].AsInt;
  {...}
end);

which is exactly what you said NOT to do, so there's that, haha.  I generally hate 'with' statements, but, well, I sorta like the way this reads.

 

I should also add that the Prepare function checks the result of sqlite3_prepare_v2() and throws an exception if there is an error with an explanation of the error.

 

I have added a link to the Rgd.SQlite3 unit here https://github.com/rgdawson/Rgd.Sqlite3/blob/main/Rgd.Sqlite3.pas

Edited May 6 by rgdawson
added