Dynamic array empty elements issue

[noszone]

I have a dynamic array of record data type, like:

Code: Pascal  [Select][+][-]

1. id:largeint,
2. empid:largeint,
3. description:string

I am fetching data from db and making output to xlsx file.

In loop process there, when writing values to array there is always "empty" elements, let say the real data length must be 700 due of an IF condition of data filtering. While total loop count increases array length up to 7000 and contains empty elements. 

But if I wisely use the increment, something like:

Code: Pascal  [Select][+][-]

1. if ..... then
2. setlength(array, length(array)+1)
3. ...write values to array
4. inc(k)

So there will be no empty elements. And I can use that data very well.
So same IF condition(when taking completed array and starting output to xlsx) doesn't work well with 1st case. It looks like due of holes, the data in output xlsx file reduced to 20% only. I miss 80% of data somewhere.

If condition(just filtering array before send to xlsx):

Code: Pascal  [Select][+][-]

1. for j:=0 to Length(cert_d)-1 do begin
2. if (emp_v[i].id=cert_d[j].empid) and
3.  (cert_v[cert_count].id=cert_d[j].id) then begin
4. ...Output to xlsx

j-the array loop, i-employee loop, cert_count-emp certificates type loop.

This is really strange, due loop goes to each element, even if the length of array is 7000 with some empty elements...

What problem this can be?

[TRon]

I had two large coffee now (to try and wake up) and I'm sorry to say that I still don't understand your explanation(s) or what it is that you are trying to achieve. Perhaps I just woke up dumb this morning (happens on occasion, not your fault) 

So far I got:
1 database with let's say million's of record
2. database is filtered so that there are 700 filtered results.
3. you have a loop that counts to 7000 and with each iteration increases the length of an array
4. as a consequence your array contains empty elements ?

I fail to see the connection between 700 database records vs 7000 array elements and what you are trying to copy and what exactly from where to where . Your code-snippets are not very helpful in that regards.

I have no idea what the current situation is, let alone what the problem could be.

Could you please share some more code and explain the connection between the different items that you speak of ? (nr of db records, nr of array elements, j-the array loop, i-employee loop, cert_count-emp certificates type loop).

[howardpc]

As with all posts asking for help debugging an app, the simplest compilable project that shows the problem is essential for others to comment intelligently.
Otherwise the thread becomes random shots in the dark, guesses (which might be inspired) mixed with contributions which later are seen to be irrelevant simply because the original question is imprecise or incomplete.
Code snippets shown are usually less than a few percent of the entire project's code, and forum members are expected to somehow crystal ball what is not shown.

[Thaddy]

3. you have a loop that counts to 7000 and with each iteration increases the length of an array

No, that is not how real databases work. They work more akin to Pascal sets. Filtering can be done in O(1). And Pascal is the perfect way to demonstrate that since it has native set support. Although limited in size, Pascal sets demonstrate the principle. Together with Venn diagrams to visualize filtering. No loops. In theory. Filters are masks, not loops.

[noszone]

I am sorry for the luck of code, here is the function which generates array:

Code: Pascal  [Select][+][-]

1. function TForm1.GetDataFromDb_Cert(SelEmp:DataTypeEmp;SelItems:DataType):DocumentDataType;
2. var i,j,k,count:integer;
3. typeid,empid,issued,expired:TField;
4. begin
5. k:=0;
6. SQLQuery5.Close;
7. SQLQuery5.Clear;
8. SQLQuery5.sql.add('select EMPLOYEEID,EMPLICENSETYPEID,ISSUEDDATE,EXPIRYDATE '+
9. 'from spectwosuite.emplicense where '+
10. 'internalstatus<>70 and internalstatus<>0');
11. SQLQuery5.Open;
12. SQLQuery5.Last;
13. SQLQuery5.First;
14. empid:=SQLQuery5.FieldByName('EMPLOYEEID');
15. typeid:=SQLQuery5.FieldByName('EMPLICENSETYPEID');
16. issued:=SQLQuery5.FieldByName('ISSUEDDATE');
17. expired:=SQLQuery5.FieldByName('EXPIRYDATE');
18.  
19. for i:=0 to length(SelEmp)-1 do begin         //main emp. loop
20.  
21. for count:=0 to length(SelItems)-1 do begin    //certs
22.  
23. SQLQuery5.First;
24. for j:=0 to SQLQuery5.RecordCount-1 do begin
25. if (typeid.AsLargeInt=SelItems[count].id) and
26. (empid.AsLargeInt=SelEmp[i].id) then begin
27. SetLength(cert_d,Length(cert_d)+1);
28. cert_d[k].id:=typeid.AsLargeInt;
29. cert_d[k].empid:=empid.AsLargeInt;
30. cert_d[k].description:=SelItems[count].description;
31. cert_d[k].date_issued:=issued.asstring;
32. cert_d[k].date_expired:=expired.asstring;
33. debug_file.WriteAnsiString(inttostr(cert_d[k].empid)+' '+ inttostr(cert_d[k].id)+
34. selEmp[i].surname+cert_d[k].description+' '+cert_d[k].date_expired);
35. debug_file.WriteByte(13);
36. inc(k);
37.   end;
38.  
39. SQLQuery5.Next;
40. end;
41.  
42. end;
43.  
44. end;
45. result:=cert_d;
46. end;

As you can see, here I've used an increment, so no holes in arrays(the program now working 100%, but I want to know why with empty elements it doesn't work properly).

And here is types:

Code: Pascal  [Select][+][-]

1. type
2.  
3.   DocumentDataRecord=record
4.     id:largeint;
5.     empid:largeint;
6.     description:string;
7.     date_issued:string;
8.     date_expired:string;
9.   end;
10.   DocumentDataType=array of DocumentDataRecord;
11.  
12.   DataRecord=record
13.     id:largeint;
14.     description:string;
15.   end;
16.   DataType=array of DataRecord;
17.  
18.   DataRecordEmp=record
19.     id:largeint;
20.     surname:string;
21.     forename:string;
22.     position:string;
23.   end;
24.   DataTypeEmp=array of DataRecordEmp;

So after function GetDataFromDb_Cert array of records goes to above IF condtion and goes to excel:

Code: Pascal  [Select][+][-]

1. for i:=0 to length(emp_v)-1 do begin
2.  
3. MyWorksheet.WriteText(i+ExcelRowShift, 0, emp_v[i].surname+' '+emp_v[i].forename);
4. MyWorksheet.WriteBorders(i+ExcelRowShift, 0, [cbNorth, cbSouth,cbWest, cbEast]);
5. MyWorksheet.WriteText(i+ExcelRowShift, 1, emp_v[i].position);
6. MyWorksheet.WriteBorders(i+ExcelRowShift, 1, [cbNorth, cbSouth,cbWest, cbEast]);
7. MyWorksheet.WriteRowHeight(ExcelRowShift-1,ExcelHeaderRowHeight,suMillimeters);
8.  
9. for cert_count:=0 to Length(cert_v)-1 do begin
10.  
11. if i=0 then begin
12. MyWorksheet.WriteWordwrap(ExcelRowShift-1,cert_count+ExcelColShift,true);
13. MyWorksheet.WriteHorAlignment(ExcelRowShift-1, cert_count+ExcelColShift, haCenter);
14. MyWorksheet.WriteVertAlignment(ExcelRowShift-1, cert_count+ExcelColShift, vaCenter);
15. MyWorksheet.WriteFontStyle(i+ExcelRowShift-1, cert_count+ExcelColShift,[fssBold]);
16. MyWorksheet.WriteBorders(i+ExcelRowShift-1, cert_count+ExcelColShift, [cbNorth, cbSouth,cbWest, cbEast]);
17. MyWorksheet.WriteText(i+ExcelRowShift-1, cert_count+ExcelColShift, cert_v[cert_count].description);
18. end;
19.  
20. for j:=0 to Length(cert_d)-1 do begin
21. if (emp_v[i].id=cert_d[j].empid) and (cert_v[cert_count].id=cert_d[j].id) then begin
22. MyWorksheet.WriteText(i+ExcelRowShift, cert_count+ExcelColShift, cert_d[j].date_expired);
23. MyWorksheet.WriteBorders(i+ExcelRowShift, cert_count+ExcelColShift, [cbNorth, cbSouth,cbWest, cbEast]);
24.  
25. if length(cert_d[j].date_expired)<>0 then begin
26.       cmp:=comparedate(strtodate(cert_d[j].date_expired),test_date);
27.       if cmp<0 then
28.       MyWorksheet.WriteBackgroundColor(i+ExcelRowShift,cert_count+ExcelColShift, clRed);
29.     end;
30. {Inc(FProgress);
31. StatusBar1.Panels.Items[1].Text := IntToStr(FProgress); //init a redraw
32. Application.ProcessMessages;}
33. end
34. else begin
35. MyWorksheet.WriteBorders(i+ExcelRowShift, cert_count+ExcelColShift, [cbNorth, cbSouth,cbWest, cbEast]);
36. end;
37.  
38. end;
39.  
40. end;
41.  
42. end;
43. MyWorkbook.WriteToFile(MyDir + 'CO-HSE-Matrix.xls', sfExcel8, true);

[noszone]

I fail to see the connection between 700 database records vs 7000 array elements and what you are trying to copy and what exactly from where to where . Your code-snippets are not very helpful in that regards.

Let imagine in db you have 10 employees and they have in certificates tables about 100 documents. So when you doing loop over these records:

Code: Pascal  [Select][+][-]

1. for i... do begin
2. Setlength(myarray,Length(myarray)+1)
3. ...retrive data from db
4. IF condtin .... then
5. write values to array[i]  // current i value can be 230 when IF condtion becomes true
6. end;

So we have array like [empty,empty,empty,empty,empty,empty,empty,empty,empty,empty,DATA,empty,empty,empty,empty,empty,empty,empty,empty,empty,empty,data].

[TRon]

Thank you for the additional information/code noszone.

As you can see, here I've used an increment, so no holes in arrays(the program now working 100%, but I want to know why with empty elements it doesn't work properly).

That depends on what empty elements you meant exactly. Are you referring to SelEmp and SelItems ?

And what do you mean exactly with "it does not work properly" ?

Are you referring to your result parameter DocumentDataType, your cert_d array (is that a global variable perhaps because it is missing from your code-snippets ?) or are you referring to (visual) results with regards to your datasheet ?

Let imagine in db you have 10 employees and they have in certificates tables about 100 documents. So when you doing loop over these records:
 <snip>
So we have array like [empty,empty,empty,empty,empty,empty,empty,empty,empty,empty,DATA,empty,empty,empty,empty,empty,empty,empty,empty,empty,empty,data].

Yeah, I understand now but what I do not understand if that is the behaviour you want/seek or if those are the empty elements you wish to know of as of why they are there.


Please keep in mind that we are not mind-readers. We do not have the same source-code in front of us (only some small snippets) We do not have the same data from the database that you use, no fpworksheet and certainly not your mindset. Nobody in his/her right mind wil try to replicate and/or fill in the missing bits to try and replicate what you have in front of you.

So if you have questions or explanations then please be specific in your wording.

[dseligo]

You code looks ok, but it is hard to be sure from code snippets. Maybe you have error in some code you didn't show.
You can prepare and post test data which shows problem (of course, with changed names and other sensitive data).
Or you'll have to debug it step by step.
Find employee who misses data in Excel. Then see what certificates he should have. Then go step by step for just that employee and see where data start "missing".

One more thing.
I don't know if you use this data somewhere else, but if you have all this data in database maybe you don't need to fill all these arrays.
For me it would be much simpler to select data from database, prepared for excel.

Something like that:

Code: Pascal  [Select][+][-]

1. var LastEmpID: Integer = -1;
2. ...
3. MyWorksheet.WriteText('...write headers...');
4. // if there are employees without license and you need to show them, then you need
5. // left outer join
6. With SQLQuery5 do begin
7.   SQL.Text := 'select emp.id as empid, emp.surname, emp.forename, emp.position, ' +
8.                   '       lic.id as licid, lic.description, ' +
9.                   '       emplic.ISSUEDDATE, emplic.EXPIRYDATE ' +
10.                   'from spectwosuite.employee emp, spectwosuite.license lic, spectwosuite.emplicense emplic'+
11.                   'where emplic.internalstatus<>70 and emplic.internalstatus<>0 ' +
12.                   'and emp.id = emplic.EMPLOYEEID ' +
13.                   'and lic.id = emplic.EMPLICENSETYPEID ' +
14.                   'order by emp.id, lic.id'
15.                   );
16.   Open;
17.   While not Eof do begin
18.     If LastEmpID <> FieldByName('empid').AsInteger then begin
19.       LastEmpID := FieldByName('empid').AsInteger;
20.       inc(CurrentRow);
21.       MyWorksheet.WriteText('.... employee name, surname, id, position, ...');
22.     end;
23.  
24.     // find column where to write EXPIRYDATE
25.     For i := 0 to cert_count - 1 do
26.       If ... then
27.         MyWorksheet.WriteText('... EXPIRYDATE  ...');
28.   end;
29.   // write excel to file
30. end;

[Thaddy]

The loop should be handled by query (which uses masks) not as a loop, I explained that. Looping is a great way to show native code is almost always much slower than an SQL database that is properly written. (and that is not a joke but fact)

Adapt your query.

[balazsszekely]

@noszone
When retrieving a lot of records with TSQLQuery, your heap memory may become full, leading to unpredictable behaviour. To solution is simple, set the unidirectional property to true. Please try the following code, before anything else:

Code: Pascal  [Select][+][-]

1. //...
2. SQLQuery5.UniDirectional := True; //add this
3. SQLQuery5.Open;
4. //SQLQuery5.Last; //this is wrong, not needed
5. SQLQuery5.First
6. while not SQLQuery5.EOF do
7. begin
8.   //export here
9.   SQLQuery5.Next
10. end;

[noszone]

Thank you all for heads up, I've to test all of these things. Agree, the sql query isn't near perfect. Have to say, this program was designed several years ago, and now needs new functionality. With your warming help everything is possible .

[noszone]

@noszone
When retrieving a lot of records with TSQLQuery, your heap memory may become full, leading to unpredictable behaviour.

Thanks for good advise!

[TRon]

One more thing.
I don't know if you use this data somewhere else, but if you have all this data in database maybe you don't need to fill all these arrays.
For me it would be much simpler to select data from database, prepared for excel.

Exactly my thought as well. E.g. a master detail relationship could be helpful as well (in case the data is stored amongst several databases).

On the other hand, if the databases are huge i can understand trying to want to peed up especially this search function although I would probably do it the other way around so that the database is iterated only once (and arrays multiple times).

[dseligo]

One more thing.
I don't know if you use this data somewhere else, but if you have all this data in database maybe you don't need to fill all these arrays.
For me it would be much simpler to select data from database, prepared for excel.

Exactly my thought as well. E.g. a master detail relationship could be helpful as well (in case the data is stored amongst several databases).

On the other hand, if the databases are huge i can understand trying to want to peed up especially this search function although I would probably do it the other way around so that the database is iterated only once (and arrays multiple times).

In example I posted database is iterated only once. I would improve searching column for 'expirydate': I'd put column index in 'license' table, so it could be also extracted from database. Then only loop would be while, to iterate through records.

[Nicole]

There may be another topic: a handbrake

setlength(array, length(array)+1)

Setting the length one by one reduces the performance of your program drasticallly.
Work with blocks of e.g. 500 and delete the last part of your block, when the work is done.