3
\$\begingroup\$

An error in a live environment was detected when trying to convert string geo coordinates into a double.

The current culture (hu-HU) uses comma instead of point for the decimals and that produced an exception when trying to parse Double.Parse(47,545874) That's why I'm using CultureInfo.InvariantCulture and the reason to wrap the conversion up in a try/catch statement.

Is there a better way to handle these exceptions?

public void UpdateJobGeoCoordinates(Job job)
{
    var address = _kernel.GetKernel();

    var gpsCoordinate = _locator.GetGpsCoordinates(address);

    try
    {
        job.Lat = Double.Parse(gpsCoordinate.Item1, CultureInfo.InvariantCulture);
        job.Lng = Double.Parse(gpsCoordinate.Item2, CultureInfo.InvariantCulture);
    }
    catch (FormatException)
    {
        ResetCoordinates(job);
        Logger.Warn("Geo location issue for Job [{0}] , Format error when convert '{1}', '{2}' to a Double.", job.ERPServiceOrderId, gpsCoordinate.Item1, gpsCoordinate.Item2);
    }
    catch (OverflowException)
    {
        ResetCoordinates(job);
        Logger.Warn("Geo location issue for Job [{0}] , Overflow when convert '{1}', '{2}' to a Double.", job.ERPServiceOrderId, gpsCoordinate.Item1, gpsCoordinate.Item2);
    }
    catch (Exception ex)
    {
        ResetCoordinates(job);
        Logger.Warn("Geo location issue for Job [{0}] , Exception message: {1}", job.ERPServiceOrderId, ex.Message);
    }
}
\$\endgroup\$
7
  • \$\begingroup\$ Could you provide more background with your code to allow for a decent review? What's it's purpose, how is it used, etc. \$\endgroup\$ – Mast May 5 '15 at 15:26
  • \$\begingroup\$ Why don't you use Double.TryParse? \$\endgroup\$ – BCdotWEB May 7 '15 at 7:23
  • \$\begingroup\$ If _locator always returns Item1 and Item2 (Tuple?) formatted with the invariant culture number format info rules then you should parse by those rules too (as you are doing). TryParse could help if _locator can return invalid doubles. \$\endgroup\$ – RobH May 7 '15 at 9:03
  • \$\begingroup\$ Thanks @Jamal for your changes. My only concern is about the original purpose of my question; It's not just about "updating geo coordinates", it's more about parsing values and capture possible exceptions. I wonder if the title fits with that. Looking forward for your thoughts. \$\endgroup\$ – tribet May 8 '15 at 7:00
  • \$\begingroup\$ @BCdotWEB, I don't use TryParse because I want to log different messages depending on the type of exception. How could I do this more effectively if I would need to run this code in more places of my solution? \$\endgroup\$ – tribet May 8 '15 at 7:09
1
\$\begingroup\$

For what it's worth I think you've over complicated this. It seems to me that you care if you succeed or not and then want to pass as much information on to someone who can figure out what went wrong. The only 2 lines that are in the try block are parsing to doubles so just catch Exception and beef up the information included.

public void UpdateJobGeoCoordinates(Job job)
{
    var address = _kernel.GetKernel();

    var gpsCoordinate = _locator.GetGpsCoordinates(address);

    try
    {
        job.Lat = Double.Parse(gpsCoordinate.Item1, CultureInfo.InvariantCulture);
        job.Lng = Double.Parse(gpsCoordinate.Item2, CultureInfo.InvariantCulture);
    }
    catch (Exception ex)
    {
        ResetCoordinates(job);
        Logger.Warn("Geo location issue for Job [{0}] , Exception details: {1}", job.ERPServiceOrderId, ex);
    }
}

Note that Exception.ToString() is so much more useful than just the message.

You'll get the type of the exception, the message and the stack trace.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.