A Question of Style

[rgdawson 7]

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 9]

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 7]

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 415]

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