Posting wrong record information to table

Question

While looping through my customer table and posting latitude and longitude values based on an address, the first set of lat/lng values are posting to records number 1 and 2, and then subsequently every record thereafter is off by one. When I step through in Debug mode I see that the values for the first record are still there on my second iteration. After that, it corrects itself, but every records values are for the address or record above it. Why?

Here's my code:

procedure TViewMaps.StartBtnClick(Sender: TObject);
var
  iRecs, i : Integer;
  Location : TLocation;
begin
  ViewMaps := TViewMaps.create(self, MapAddress);
  Customer.Open;
  iRecs:= Customer.RecordCount;
  i := 0;
  While Not Customer.EOF do
    begin
      i := i + 1;
      Customer.Edit;
      MapAddress := CustomerSAddress1.AsString + ' ' + CustomerSAddress2.AsString + ' ' + CustomerSAddress3.AsString + ' ' + CustomerSAddress4.AsString + ', ' + CustomerSCity.AsString + ', ' + CustomerSState.AsString + ' ' + CustomerSZip.AsString;

      fAddress := StringReplace(StringReplace(Trim(MapAddress), #13, ' ', [rfReplaceAll]), #10, ' ', [rfReplaceAll]);
      Location := GetGeoCode(fAddress);

      Customerlat.AsString := Location.Lat;
      Customerlng.AsString := Location.Lng;

      StatusBar1.SimpleText:= 'Update Geocode for address ' + ' [Count ' + IntToStr(i) + ' of ' + IntToStr(iRecs) + ']';
      Sleep(3000);
      StatusBar1.Refresh;
      Customer.Next;
    end;
end;

Okay, I modified the code based on your suggestion (not sure I'm doing it the right way), but I'm getting the exact same result. This program is just a run once application to populate the lat/lng values to our database, and the delay is because I was running into a Google query limit and the app was posting to the database before the results were back from Google, so I had to slow it down.

Here's the updated code:

procedure TViewMaps.StartBtnClick(Sender: TObject);
begin
  Customer.Open;
  iRecCount:= Customer.RecordCount;
  iCurRec := 0;
  Customer.First;
  if Not Customer.EOF then Timer1.Enabled := True;
end;

procedure TViewMaps.OnTimer(Sender: TObject);
begin
  iCurRec := iCurRec + 1;
  //ShowMessage('I am here and iCurRec = ' + inttostr(iCurRec));
  Customer.Edit;
  // Load full customer address into MapAddress
  MapAddress := CustomerSAddress1.AsString + ' ' + CustomerSAddress2.AsString + ' ' + CustomerSAddress3.AsString + ' ' + CustomerSAddress4.AsString + ', ' + CustomerSCity.AsString + ', ' + CustomerSState.AsString + ' ' + CustomerSZip.AsString;
  fAddress := StringReplace(StringReplace(Trim(MapAddress), #13, ' ', [rfReplaceAll]), #10, ' ', [rfReplaceAll]);
  // Get Longitude and Latitude from Google Maps
  Location := GetGeoCode(fAddress);
  // Populate lat and lng fields in Customer table
  Customerlat.AsString := Location.Lat;
  Customerlng.AsString := Location.Lng;
  // Post record to Customer table
  Customer.Post;
  StatusBar1.SimpleText:= 'Update Geocode for address ' + ' [Count ' + IntToStr(iCurRec) + ' of ' + IntToStr(iRecCount) + ']';
  StatusBar1.Refresh;
  // Grab the next record in the Customer table
  Customer.Next;
  if Customer.EOF then Timer1.Enabled := False;
end;

Ok... here's the codeAddress javascript

''+
'  function codeAddress(address) { '+
'    if (geocoder) {'+
'      geocoder.geocode( { address: address}, function(results, status) { '+
'        if (status == google.maps.GeocoderStatus.OK) {'+
'          map.setCenter(results[0].geometry.location);'+
'          var myLatlng = new google.maps.LatLng( results[0].geometry.location.lat(), results[0].geometry.location.lng()); '+
'          var marker = new google.maps.Marker({ '+
'            position: myLatlng, '+
'            title: "", '+
'            map: map '+
'          }); '+
'        markersArray.push(marker); '+
'        document.getElementById("hiddenlat").value = myLatlng.lat(); '+
'        document.getElementById("hiddenlng").value = myLatlng.lng(); '+
' '+
'        } else {'+
'            document.getElementById("hiddenlat").value = "error"; '+
'            document.getElementById("hiddenlng").value = "error"; '+
'           alert("Geocode was not successful for the following reason: " +    status);'+
'        }'+
'      });'+
'    }'+
'  }'+
''+

And here's the Delphi code:

constructor TViewMaps.create(AOwner: TComponent; AAddress: string);
begin
  inherited create(AOwner);
  fAddress := AAddress; // fAddress is now stored to form variable
end;

procedure TViewMaps.LoadGoogleApi;
var
  aStream: TMemoryStream;
begin
  WebBrowser1.Navigate('about:blank'); //Set the location to an empty page
  MemoAddress.Lines.Text := '1600 Amphitheatre Parkway, Mountain View, CA 94043';
  if Assigned(WebBrowser1.Document) then
  begin
    aStream := TMemoryStream.Create; //create a TStream to load the Page from the string
    try
      aStream.WriteBuffer(Pointer(HTMLStr)^, Length(HTMLStr));
      aStream.Seek(0, soFromBeginning);
      (WebBrowser1.Document as IPersistStreamInit).Load(TStreamAdapter.Create(aStream));
    finally
      aStream.Free;
    end;
    HTMLWindow2 := (WebBrowser1.Document as IHTMLDocument2).parentWindow;
  end;

  while WebBrowser1.ReadyState <> READYSTATE_COMPLETE do // wait for google
  begin
    sleep(0);
    application.processmessages;
  end;
end;

function TViewMaps.GoogleApiReady: boolean;
begin
  result := (HTMLWindow2 <> nil);
end;

procedure TViewMaps.ExecuteScript(AScript: string);
begin
  HTMLWindow2.execScript(AScript, 'JavaScript');
end;

function TViewMaps.GetElementByID(AElementID: string): IHTMLElement;
begin
  result := (WebBrowser1.Document as IHTMLDocument3).getElementByID(AElementID);
end;

function TViewMaps.GetElementValue(ElementID: string): string;
var
  HtmlElement: IHTMLElement;
begin
  HtmlElement := GetElementByID(ElementID);
  result := HtmlElement.getAttribute('value', 0);
end;

procedure RemoveInvalidGeoLookupChars(var AString: string);
begin
  AString := StringReplace(StringReplace(Trim(AString), #13, ' ', [rfReplaceAll]), #10, ' ', [rfReplaceAll]);
  // remove invalid chars
  AString := StringReplace(AString, #39, #32, [rfReplaceAll]);  // single quotes
  AString := StringReplace(AString, #34, #32, [rfReplaceAll]);  // double quotes
end;

procedure TViewMaps.FormShow(Sender: TObject);
var
  Location: TLocation;
begin
  MapAddress := '1600 Amphitheatre Parkway' + ', ' + 'Mountain View' + ', ' + 'CA' + ' ' + '94043';
  ViewMaps := TViewMaps.create(self, MapAddress);
  LoadGoogleApi;
  address := MemoAddress.Lines.Text;
  fAddress := StringReplace(StringReplace(Trim(address), #13, ' ', [rfReplaceAll]), #10, ' ', [rfReplaceAll]);
  Location := GetGeoCode(fAddress);
  LatitudeEdit.Text := Location.Lat;
  LongitudeEdit.Text := Location.Lng;
end;

function TViewMaps.GetGeocode(Address: string): TLocation;
begin
  result.Lat := '0';
  result.Lng := '0';
  LatitudeEdit.text := '0';
  LongitudeEdit.text := '0';
  result.Result := 'OK';
  application.processmessages;
  RemoveInvalidGeoLookupChars(address);
  application.processmessages;
  ExecuteScript(Format('codeAddress(%s)',[QuotedStr(Address)]));

  while (GetElementValue('hiddenlat') = '0') do
    application.processmessages;

  result.Lat := GetElementValue('hiddenlat');
  result.Lng := GetElementValue('hiddenlng');
end;

procedure TViewMaps.StartBtnClick(Sender: TObject);
var
  iRecCount, iCurRec: integer;
  Location: TLocation;
  fAddress, MapAddress: string;
begin
    Customer.open;
    Customer.first;
    iRecCount := Customer.RecordCount;
    iCurRec := 0;
    while not Customer.eof do
    begin
      inc(iCurRec);
      fillchar(Location, sizeof(Location), 0);
      MapAddress := CustomerSAddress1.asstring+' '+CustomerSAddress2.asstring+' '+CustomerSCity.asstring+', '+CustomerSState.asstring+' '+CustomerSZip.asstring;
      fAddress := StringReplace(StringReplace(Trim(MapAddress), #13, ' ', [rfReplaceAll]), #10, ' ', [rfReplaceAll]);
      fillchar(Location, sizeof(Location), 0);
      Location := GetGeocode(fAddress);
      if (Location.lat <> 'error') and (Location.lat <> '0') then
      begin
        Customer.edit;
        CustomerLat.AsString := Location.Lat;
        CustomerLng.AsString := Location.Lng;
        Customer.Post;
      end;
      Statusbar1.SimpleText := 'Update Geocode for address ' + ' [Count ' + IntToStr(iCurRec) + ' of ' + IntToStr(iRecCount) + ']';
      application.processmessages;
      sleep(2000); // adjust to not exceed Google API query limit
      Customer.next;
    end;
end;

Answer 1

Users of SO should know that this is a continuation to one of OP's previous questions - Getting Latitude Longitude from GoogleMaps in TWebBrowser

I created a test using the code below. My customer table field names are different, but you'll get the idea.

Notice to keep the time between Customer.edit and Customer.post as narrow as possible, to avoid any premature posts. Also, make sure that Customer table doesn't have any events that would cause an interruption during your loop. Only edit/post if a valid geocode has been returned. And for good measure, the Location record is initialized at each iteration.

procedure TForm2.StartBtnClick(Sender: TObject);
var
  iRecCount,
  iCurRec: integer;
  Location: TLocation;
  fAddress, MapAddress: string;
begin
    Customer.open;
    Customer.first;
    iRecCount := Customer.RecordCount;
    iCurRec := 0;
    while not Customer.eof do
    begin
      inc(iCurRec);
      fillchar(Location, sizeof(Location), 0);
      MapAddress := CustomerAddress.asstring+' '+CustomerAddress2.asstring+' '+CustomerCity.asstring+', '+CustomerState.asstring+' '+CustomerZip.asstring;
      fAddress := StringReplace(StringReplace(Trim(MapAddress), #13, ' ', [rfReplaceAll]), #10, ' ', [rfReplaceAll]);
      fillchar(Location, sizeof(Location), 0);
      Location := form1.GetGeocode(fAddress);
      if (Location.lat <> 'error') and (Location.lat <> '0') then
      begin
        Customer.edit;
        CustomerLatitude.AsString := Location.Lat;
        CustomerLongitude.AsString := Location.Lng;
        Customer.Post;
      end;
      Statusbar1.SimpleText := 'Update Geocode for address ' + ' [Count ' + IntToStr(iCurRec) + ' of ' + IntToStr(iRecCount) + ']';
      application.processmessages;
      sleep(1000); // adjust to not exceed Google API query limit
      Customer.next;
    end;
end;   

Also, modify your codeAddress Javascript function to clear the hidden page values.

''+
'  function codeAddress(address) { '+
'    document.getElementById("hiddenlat").value = "0"; '+
'    document.getElementById("hiddenlng").value = "0"; '+
'    if (geocoder) {'+
'      geocoder.geocode( { address: address}, function(results, status) { '+
'        if (status == google.maps.GeocoderStatus.OK) {'+
'          map.setCenter(results[0].geometry.location);'+
'          var myLatlng = new google.maps.LatLng( results[0].geometry.location.lat(), results[0].geometry.location.lng()); '+
'          var marker = new google.maps.Marker({ '+
'            position: myLatlng, '+
'            title: "", '+
'            map: map '+
'          }); '+
'        markersArray.push(marker); '+
'        document.getElementById("hiddenlat").value = myLatlng.lat(); '+
'        document.getElementById("hiddenlng").value = myLatlng.lng(); '+
' '+
'        } else {'+
'            document.getElementById("hiddenlat").value = "error"; '+
'            document.getElementById("hiddenlng").value = "error"; '+
'           alert("Geocode was not successful for the following reason: " +    status);'+
'        }'+
'      });'+
'    }'+
'  }'+
''+

Answer 2

Not sure where in your loop you are making these observations, but in the code that was originally in your q, the changes to the current record will not get posted until your call to Customer.Next which will post any changes before moving the table cursor to the next record. But you should not rely on this behaviour, especially as you are only calling Customer.Next after a delay.

Try this instead:

  Customerlng.AsString := Location.Lng;
  Customer.Post;

I would not be at all surprised if it turns out that the problem you're having turns out to be caused by some artefact of the delay you say you're having to introduce to avoid calling GetGeoLocation too frequently. A way to eliminate that as the cause is to use a test version of your original loop with no delay (Sleep()) in it whatsoever and a replacement GetGeoCode routine which simply returns some unique values each time it is called. This replacement GetGeoCode routine could simply increment the Lat and Lng members of a record each time it is called and return the updated record.

This simplified test version should work correctly. If it doesn't, your code must be doing something you are not telling us about and only you can debug it. If on the other hand it does work correctly, you need to come up with a better way of avoiding calling the real GetGeoCode too frequently. In particular it should not

a) call Sleep() in your app's main gui thread, which is what you were doing in the original version of your q; and

b) attempt to execute entirely within an OnClick handler.

If you really must introduce a delay, do something like

• Add a TTimer to your form with a delay of 3000 or whatever.

• In your StartBtnClick, simply call Customer.First, check for Customer.Eof and if it's false, activate the tttimer. Then exit.

• In its OnTimer, update the current Customer record, call Customer.Post, do your status bar refresh, call Customer.Next, check for Customer.Eof and cancel the ttimer if Eof is true so that its OnTimer won't be called again.

You should protect your OnTimer against re-entrancy (i.e. OnTimer being called again before it has finished executing). Have a boolean flag UpdateExecuting on your form, test for it being true on entry to OnTimer and exit immediately if it is, otherwise set it to true, then set it to false in the finally section of a try..finally in the remainder of your OnTimer. Or, you could disable the TTimer on entry to its OnTimer event and re-enable it in a finally section.

If you prefer not to use a TTimer, you could do what I've suggested for its OnTimer in an Application.OnIdle handler.

Best of all would be to do the updates in a background thread, but how easy it would be to retrieve the updates into the Customer instance in your gui thread depends on what kind of TDataSet your Customer table is.

Btw: your Customer table isn't using an index that includes Customerlat or Customerlng is it?

Also btw: your q doesn't contain a proper MCVE, which is what a question of this type should contain, because without one, there is no way for readers to reproduce the problem. Without one, I think it is likely to attract votes to close.