Why does this code only work the second time it is run?

Question

In my mapping app, when I populate a table with all the data about a location EXCEPT the coordinates (Latitude and Longitude) -- which I don't expect the user to know/provide), those values are determined later programmatically when the map that the data represents is loaded in the app for the first time.

When I do load the existing map from the main form, the code is this:

private void loadExistingMapToolStripMenuItem_Click(object sender, EventArgs e)
{
    bool pushpinsAdded = false;
    RemoveCurrentPushpins();
    using (var frmLoadExistingMap = new mdlDlgFrm_LoadExistingMap())
    {
        if (frmLoadExistingMap.ShowDialog() == DialogResult.OK)
        {
            foreach (Pushpin pin in frmLoadExistingMap.Pins)
            {
                this.userControl11.myMap.Children.Add(pin);
                pushpinsAdded = true;
            }
        }
    }
    if (pushpinsAdded)
    {
        RightsizeZoomLevelForAllPushpins();
    }
    lblMapName.Text = currentMap;
    lblMapName.Visible = true;
}

The code in the "Load Existing Map" form (frmLoadExistingMap) determines the coordinates for the address. If the coordinates exist already, great; otherwise, a REST call is made to get those values:

private async void AddPushpinToListOfPushpins(string location, string fullAddress, string mapDetailNotes, double latitude, double longitude, string pushpinColor)
{
    iContentCounter = iContentCounter + 1;
    string toolTip = string.Empty;
    // if already have the record, including the coordinates, no need to make the REST call
    if ((latitude != 0.0) && (longitude != 0.0))
    {
        if (mapDetailNotesExist(location))
        {
            toolTip = String.Format("{0}{1}{2}{1}{3},{4}{1}{5}", location, Environment.NewLine, fullAddress, latitude, longitude, mapDetailNotes.Trim());
        }
        else
        {
            toolTip = String.Format("{0}{1}{2}{1}{3},{4}", location, Environment.NewLine, fullAddress, latitude, longitude);
        }
        var _mapLocation = new Microsoft.Maps.MapControl.WPF.Location(latitude, longitude);
        this.Pins.Add(new Pushpin()
        {
            Location = _mapLocation,
            ToolTip = toolTip,
            Content = iContentCounter,
            Background = new SolidColorBrush(GetColorForDesc(pushpinColor))
        });
    }
    else
    {
        // from https://stackoverflow.com/questions/65752688/how-can-i-retrieve-latitude-and-longitude-of-a-postal-address-using-bing-maps
        var request = new GeocodeRequest();
        request.BingMapsKey = "heavens2MurgatroidAndSufferinSuccotash";
        request.Query = fullAddress;

        var result = await request.Execute();
        if (result.StatusCode == 200)
        {
            var toolkitLocation = (result?.ResourceSets?.FirstOrDefault())
                    ?.Resources?.FirstOrDefault()
                    as BingMapsRESTToolkit.Location;
            var _latitude = toolkitLocation.Point.Coordinates[0];
            var _longitude = toolkitLocation.Point.Coordinates[1];
            var mapLocation = new Microsoft.Maps.MapControl.WPF.Location(_latitude, _longitude);                
            this.Pins.Add(new Pushpin() 
            { 
                Location = mapLocation,
                ToolTip = String.Format("{0}{1}{2}{1}{3},{4}{1}{5}", location, Environment.NewLine, fullAddress, _latitude, _longitude, mapDetailNotes),
                Content = iContentCounter,
                Background = new SolidColorBrush(GetColorForDesc(pushpinColor))
            });
            UpdateLocationWithCoordinates(location, _latitude, _longitude);
        }
    }
}

When the "else" block in the code directly above is reached (when the coordinates do not yet exist in the database), the pushpins are not added the first time. However, if I immediately run the same code again, it works - even without stopping and restarting the app.

I know the problem has something to do with the async code, but why does it never (so far) work the first time, but always (so far) works the second time?

And more importantly, is there a way I can see to it that it works the first time?

UPDATE

For Reza Aghaei:

private void UpdateLocationWithCoordinates(string location, double latitude, double longitude)
{ 
    String query = "UPDATE CartographerDetail " +
                   "SET Latitude = @Latitude, " +
                   "Longitude = @Longitude " +
                   "WHERE FKMapName = @FKMapName AND LocationName = @LocationName";
    try
    {
        var con = new SqliteConnection(connStr);
        con.Open();

        SqliteCommand cmd = new SqliteCommand(query, con);
        // FKMapName and LocationName are for the WHERE clause
        cmd.Parameters.AddWithValue("@FKMapName", mapName);
        cmd.Parameters.AddWithValue("@LocationName", location);
        // Updated values
        cmd.Parameters.AddWithValue("@Latitude", latitude);
        cmd.Parameters.AddWithValue("@Longitude", longitude);
        cmd.ExecuteNonQuery();
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
}

Here's the "Load Existing Map" form:

Its "Load Selected Map" button click does this:

private void btnLoadSelectedMap_Click(object sender, EventArgs e)
{
    string latitudeAsStr = string.Empty;
    string longitudeAsStr = string.Empty;
    List<MapDetails> lstMapDetails = new List<MapDetails>();
    MapDetails md;
    . . . 
    try
    {
        Form1.currentMap = mapName; 
        string qry = "SELECT LocationName, Address1, Address2, City, StateOrSo, PostalCode, " +
                        "MapDetailNotes, Latitude, Longitude " +
                        "FROM CartographerDetail " +
                        "WHERE FKMapName = @FKMapName";
        var con = new SqliteConnection(connStr);
        con.Open();
        SqliteCommand cmd = new SqliteCommand(qry, con);
        cmd.Parameters.AddWithValue("@FKMapName", mapName);
        var reader = cmd.ExecuteReader();
        while (reader.Read())
        {
            md = new MapDetails();
            md.LocationName = reader.GetValue(LOC_NAME).ToString().Trim();
           . . .
            latitudeAsStr = reader.GetValue(LATITUDE).ToString();
            if (string.IsNullOrEmpty(latitudeAsStr))
            {
                md.Latitude = 0.0;
            }
            else
            {
                md.Latitude = Convert.ToDouble(reader.GetValue(LATITUDE).ToString());
            }
            longitudeAsStr = reader.GetValue(LONGITUDE).ToString();
            if (string.IsNullOrEmpty(longitudeAsStr))
            {
                md.Longitude = 0.0;
            }
            else
            {
                md.Longitude = Convert.ToDouble(reader.GetValue(LONGITUDE).ToString());
            }
            lstMapDetails.Add(md);
        }
        AddPushpinsToListOfPushpins(lstMapDetails);
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
    // All pushpins have been loaded
    this.Close();
}

Its Form_Closed and Close button code is:

private void mdlDlgFrm_LoadExistingMap_FormClosed(object sender, FormClosedEventArgs e)
{
    this.DialogResult = DialogResult.OK; // Would this be a problem, assuming "OK" is always okay?
}

private void btnClose_Click(object sender, EventArgs e)
{
    this.DialogResult = DialogResult.Cancel;
}

As a bonus, the AddPushpinsToListOfPushpins() code - which calls AddPushpinToListOfPushpins() in a loop - is:

private void AddPushpinsToListOfPushpins(List<MapDetails> lstMapDetails)
{
    string fullAddress;

    foreach (MapDetails _md in lstMapDetails)
    {
        fullAddress = string.Format("{0} {1} {2} {3}", _md.Address, _md.City, _md.StateOrSo, _md.PostalCode).Trim();
        AddPushpinToListOfPushpins(_md.LocationName, fullAddress, _md.MapDetailNotes, _md.Latitude, _md.Longitude, _md.PushpinColor);
    }
}

LOL - heavens2MurgatroidAndSufferinSuccotash :) Hope that is the updated fake key. If not, please edit your question. — Nelson, Mar 11 '21 at 04:13
Could you post `UpdateLocationWithCoordinates` method and also show how/when you close the `mdlDlgFrm_LoadExistingMap` dialog? — Reza Aghaei, Mar 11 '21 at 09:42
@Reza: Done, posted an Update with that code (and more). Thanks. — B. Clay Shannon-B. Crow Raven, Mar 11 '21 at 16:27

Reza Aghaei · Accepted Answer · 2021-03-12T07:40:08.297

As far as I see, the main problem in the code is calling async method without using await operator. Please consider the following points when using async/await:

async void is ok for event handlers, but not for methods that you need to call directly in code.

Change:

private async void AddPushpinToListOfPushpins(string location, string fullAddress, string mapDetailNotes, double latitude, double longitude, string pushpinColor)

to:

private async Task AddPushpinToListOfPushpins(string location, string fullAddress, string mapDetailNotes, double latitude, double longitude, string pushpinColor)

When you want to call a method which returns Task or Task<T>, you need to use await operator.

Change:

AddPushpinToListOfPushpins(_md.LocationName, fullAddress, _md.MapDetailNotes, _md.Latitude, _md.Longitude, _md.PushpinColor);

to:

await AddPushpinToListOfPushpins(_md.LocationName, fullAddress, _md.MapDetailNotes, _md.Latitude, _md.Longitude, _md.PushpinColor);

When you need to use await operator, the method which contains that line of code, should be declared as async and the return type of the method should change from void to Task (or from T to Task<T>). (The only exception is event handlers, change them to async void.)

Change:
```
private void AddPushpinsToListOfPushpins(List<MapDetails> lstMapDetails)
```
to:
```
private async Task AddPushpinsToListOfPushpins(List<MapDetails> lstMapDetails)
```

With regards to what mentioned in second point, change:

AddPushpinsToListOfPushpins(lstMapDetails);

to:

await AddPushpinsToListOfPushpins(lstMapDetails);

With regards to what mentioned in third point (and also first point), change:

private void btnLoadSelectedMap_Click(object sender, EventArgs e)

to

private async void btnLoadSelectedMap_Click(object sender, EventArgs e)

Some other points (which doesn't have anything to do with the problem, but good to fix):

When you want to close a dialog, just set the dialog result, so in btnLoadSelectedMap_Click, replace this.Close() with this.DialogResult = DialogResult.OK;, then you don't need mdlDlgFrm_LoadExistingMap_FormClosed.
When you open a connection, you are responsible to close it so any con.Open(); should be coupled by con.Close(); when you no more need the connection.

score 1 · Answer 2 · answered Mar 11 '21 at 04:24

1

do you need to set/update the location coordinates before adding the pushpins? Without being able to debug this, its hard to tell. But it feels like if you run the UpdateLocationWithCoordinates(location, _latitude, _longitude); method call before adding your pins, it will work the first time for you. My thought is, that method call runs after the Add Pins section on the first run. Which is why it runs and adds the pins successfully on the second run, everytime "so far".

answered Mar 11 '21 at 04:24

Nelson

179
4

Sometimes, at least, the coordinates will be needed, because not every location has a "human-readable" address (e.g., the location of a shipwreck). – B. Clay Shannon-B. Crow Raven Mar 11 '21 at 05:38
UpdateLocationWithCoordinates updates the database with the coordinates. Once the user creates the map with the address, and the map is loaded, when it is found that the coordinates are not in the database, the REST call is made to get them and update the table. The collection of Pins are accessible from the main form that calls that code, though, and applies the pins to the map - they just do not display the first time for some reason... – B. Clay Shannon-B. Crow Raven Mar 11 '21 at 05:51

Why does this code only work the second time it is run?

UPDATE

2 Answers2