Locating empty field groups for saving data

The aim of the code below is to see if selected fields are empty so data can be saved in that field. Since I'm making a library system, I wanted to record the basic info on one record (all the books they have taken out so the ID's of them as well as author and title) but this proves to be quite tricky and quite long to code.

I was wondering whether there's an easier way to code this instead of the very long if loop. It would also help if I could use the same solution (obviously tweaked) when displaying their loans on the form as well.

```
procedure Save;
{ procedure to locate which field groups are empty so product and loan informati
- on can be saved }
begin
if LoanRecords.ProductID1 ='' then
begin
LoanRecords.ProductID1 := ProductRecords.UniqueId;
LoanRecords.ProductTitle1 := ProductRecords.ProductTitle;
LoanRecords.ProductAuthor1 := ProductRecords.ProductAuthor;
end
else if LoanRecords.ProductID2 = '' then
begin
LoanRecords.ProductID2 := ProductRecords.UniqueId;
LoanRecords.ProductTitle2 := ProductRecords.ProductTitle;
LoanRecords.ProductAuthor2 := ProductRecords.ProductAuthor;
end
else if LoanRecords.ProductID3 = '' then
begin
LoanRecords.ProductID3 := ProductRecords.UniqueId;
LoanRecords.ProductTitle3 := ProductRecords.ProductTitle;
LoanRecords.ProductAuthor3 := ProductRecords.ProductAuthor;
end
else if LoanRecords.ProductID4 ='' then
begin
LoanRecords.ProductID4 := ProductRecords.UniqueId;
LoanRecords.ProductTitle4 := ProductRecords.ProductTitle;
LoanRecords.ProductAuthor4 := ProductRecords.ProductAuthor;
end
else if LoanRecords.ProductID5 ='' then
begin
LoanRecords.ProductID5 := ProductRecords.UniqueId;
LoanRecords.ProductTitle5 := ProductRecords.ProductTitle;
LoanRecords.ProductAuthor5 := ProductRecords.ProductAuthor;
end
else if LoanRecords.ProductID6='' then
begin
LoanRecords.ProductID6 := ProductRecords.UniqueId;
LoanRecords.ProductTitle6 := Product

Technically, there is no such thing as if-loops. Your code is just a bunch of if-else statements (sorry, this was the language police speaking)

What your code actually needs, is actually a real loop. Use an array or a list of a record, TLoanProduct and loop through it. Your current approach allows a maximum of six products in one loan, it involves a lot of code duplication which in itself is a code smell, and it could be a bit weird if the values for Product 3 were set but not Product 1.

My Delphi skills are a bit rusty, but consider changing TLoan into an object or a class instead of a pure record. That way you could have the Save method as part of the TLoan object/class.

This is some code for how I think it should look. It's been a long while since I worked with Delphi so this might not be 100% correct code, but it should get you started in the right direction.

TLoanProduct = record
    ProductID: integer;
    ProductTitle: string[80];
    ProductAuthor: string[80];
    Returned: boolean;
end;
TLoan = record
    LoanUniqueId: integer; { of loan record }
    DateDue: TDateTime; { date when products are due to be returned }
    MemberUniqueID: integer; { from MemberDetails File }
    MemberName: string[50]; { from MemberDetails File }
    MemberSecondName: string[50]; { from MemberDetails File }
    AmountBorrowed: integer; { amount of items currently on loan }
    LoanProducts: array [0..5] of TLoanProduct;
end;

...

procedure Save;
{ procedure to locate which field groups are empty so product and loan informati
 - on can be saved }
var i: integer;
begin
  for i := 0 to High(LoanRecords.LoanProducts) do
  if LoanRecords.LoanProducts[i].ProductID = '' then
  begin
    LoanRecords.LoanProducts[i].ProductID := ProductRecords.UniqueId;
    LoanRecords.LoanProducts[i].ProductTitle := ProductRecords.ProductTitle;
    LoanRecords.LoanProducts[i].ProductAuthor := ProductRecords.ProductAuthor;
    Exit; // Exit the method so that this is only applied for one element
  end;
end;

StackExchange Code Review Q#38448, answer score: 5