1
\$\begingroup\$

I just wanted to build a simple converter from an XLS Excel file to JSON. I have predefined JSON format. All fields except of two - are just simple mapping from Excel to JSON. The two fields have a special format, "|" separated values which should be converted to array items. I also need a geocode address and save latitude and longitude to corresponding fields.

  static class Program
{
    private static GoogleGeocoder GoogleGeocoder { get; set; }

    private static SettingsModel Settings { get; set; }

    static void Main(string[] args)
    {
        using (var cc = new ConsoleCopy("log.txt"))
        {
            Console.WriteLine("--- starting convert XLS data to export.json "+DateTime.Now);

            GoogleGeocoder = new GoogleGeocoder(); // default api key used

            LoadSettings(@"settings.json");
            var result = ConvertExcelToJson();
            File.WriteAllText(@"export.json", result);

            Console.WriteLine("--- converting finished. Press any key to exit");
            Console.ReadKey();
        }
    }

    private static void LoadSettings(string settingsFileName)
    {
        try
        {
            if (!File.Exists(settingsFileName))
            {
                Console.WriteLine("--- settings file was not found...");
                File.WriteAllText(settingsFileName, JsonConvert.SerializeObject(new SettingsModel(true), Formatting.Indented));
                Console.WriteLine("--- settings file was created, please edit it and start the programm again...");
                Console.ReadKey();
                Environment.Exit(0);
            }

            Settings = JsonConvert.DeserializeObject<SettingsModel>(File.ReadAllText(settingsFileName));

            Console.WriteLine("--- reading settings finished");
        }
        catch (Exception ex)
        {
            HandleAndThrowException(ex, "Cannot open/read settings file. Please check if all settings are correct", false);
            Console.ReadKey();
            Environment.Exit(0);
        }
    }

    private static string ConvertExcelToJson()
    {
        try
        {
            var nameField = Settings.FieldsMapping.Single(n => n.DestinationFieldName.Equals("Name")).SourceFieldNumber;
            var datesField = Settings.FieldsMapping.Single(n => n.DestinationFieldName.Equals("Dates")).SourceFieldNumber;
            var timesField = Settings.FieldsMapping.Single(n => n.DestinationFieldName.Equals("Times")).SourceFieldNumber;
            var zipField = Settings.FieldsMapping.Single(n => n.DestinationFieldName.Equals("PostsalCode")).SourceFieldNumber;
            var cityField = Settings.FieldsMapping.Single(n => n.DestinationFieldName.Equals("City")).SourceFieldNumber;
            var streetField = Settings.FieldsMapping.Single(n => n.DestinationFieldName.Equals("Street")).SourceFieldNumber;

            var book = new LinqToExcel.ExcelQueryFactory(Settings.FileName);

            var worksheetName = book.GetWorksheetNames().FirstOrDefault();
            if (worksheetName == null)
                throw new Exception("--- cannot find any worksheet");

            Console.Write("--- start converting [");

            var test = book
                .WorksheetRange(Settings.CellToStart, Settings.CellToEnd, worksheetName)
                .AsEnumerable()
                .Select(item =>
                {
                    try
                    {
                        return new ExportsModel
                        {
                            Name = item[nameField].Cast<string>(),
                            ActionDates =
                                GetActionDatesArray(item[datesField].Cast<string>(), item[timesField].Cast<string>()),
                            City = item[cityField].Cast<string>(),
                            Geocode =
                                GetCoordinates(item[zipField].Cast<string>(), item[cityField].Cast<string>(),
                                    item[streetField].Cast<string>()),
                            Street = item[streetField].Cast<string>(),
                            PostalCode = item[zipField].Cast<string>(),
                            LastDate = GetLastDate(item[datesField].Cast<string>())
                        };
                    }
                    catch (Exception)
                    {
                        return null;
                    }
                })
                .Where(obj => obj != null)
                .ToJSON();

            Console.WriteLine("]");

            return test;
        }
        catch (Exception ex)
        {
            HandleAndThrowException(ex, "Cannot convert data. Please check if all settings are correct", false);
            return "";
        }
    }


    private static DateTime? GetLastDate(string dates)
    {
        try
        {
            return DateTime.Parse(dates.Split('|').Select(n => n.Trim()).LastOrDefault()).AddHours(23).AddMinutes(59);
        }
        catch (Exception ex)
        {
            HandleAndThrowException(ex, "Cannot parse last date field. Value of the field: "+dates);
            return null;
        }
    }

    private static IEnumerable<ActionDate> GetActionDatesArray(string dates, string times)
    {
        List<string> timesArray;
        List<string> datesArray;

        try
        {
            timesArray = times.Split('|').Select(n => n.Trim()).ToList();
            datesArray = dates.Split('|').Select(n => n.Trim()).ToList();

        }
        catch (Exception ex)
        {
            timesArray = new List<string>();
            datesArray = new List<string>();
            HandleAndThrowException(ex, String.Format("Cannot parse date or time field. Values needed to parse: dates={0}, times={1}", dates, times));
        }

        for (var i = 0; i < datesArray.Count(); i++)
        {
            yield return new ActionDate()
            {
                Label = datesArray[i] + " " + timesArray[i] + "Uhr",
                Date = DateTime.Parse(datesArray[i]).AddHours(23).AddMinutes(59)
            };
        }
    }

    private static Geocode GetCoordinates(string zip, string city, string street)
    {
        try
        {
            IEnumerable<Address> addresses = GoogleGeocoder.Geocode(zip + " " + city + " " + street).ToList();
            var a = addresses.First().Coordinates.Latitude;

            Console.Write('.');

            return new Geocode()
            {
                Lat = addresses.First().Coordinates.Latitude,
                Lon = addresses.First().Coordinates.Longitude
            };

        }
        catch (Exception)
        {
            // try again 
            try
            {
                return GetCoordinates(zip, city, street);
            }
            catch (Exception ex)
            {
                HandleAndThrowException(ex, String.Format("The following data cannot be geocoded: ({0}-{1}-{2})", zip, city, street));
                return null;
            }
        }
    }

    private static void HandleAndThrowException(Exception ex, string customMessage, bool throwNewException = true)
    {
        Console.WriteLine();
        Console.WriteLine("*****************");
        Console.WriteLine(ex.ToString());
        Console.WriteLine("->");
        Console.WriteLine(customMessage);
        Console.WriteLine("*****************");

        if (throwNewException)
            throw new Exception("The shop will not be added to list", ex);
    }

}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Nearly 200 lines of code in one Program class is IMHO too much. To me, the Program class of a Console application should contain as little as possible, and all logic should be contained in separate, well-defined classes. Think SOLID.

I'd refactor LoadSettings and ConvertExcelToJson to a class each, for instance.


Consider not tying your feedback code logic to the Console. Instead implement something like NLog to handle all your logging; this still allows you to output to the console. Using ConsoleCopy is a really hacky solution.


Settings.FieldsMapping.Single(n => n.DestinationFieldName.Equals("Name")).SourceFieldNumber; is repeated more than a dozen times, so instead of copy-pasting it and changing one string variable, move it to a method.


ConvertExcelToJson does a lot of things. Consider splitting it up into smaller methods, each handling a specific task. Of course, first you need to convert ConvertExcelToJson to a class of its own before you can do so.


Why do a try...parse around DateTime.Parse, when you could use DateTime.TryParse?


Twice you do item[datesField].Cast<string>(), and as it turns out, twice you then do dates.Split('|').Select(n => n.Trim()). That's a waste of effort, which to me suggest you should rethink the logic in return new ExportsModel. All of those item[nameField].Cast<string>() are pretty ugly and are repeated, so they should be moved to a method. I'd advice to convert item to an intermediate format first, before creating ExportsModel.


I'm not happy with this:

catch (Exception)
{
    return null;
}

I get your logic, but throwing an exception when not being able to parse a certain value is a bad pattern, IMHO.


Instead of datesArray[i] + " " + timesArray[i] + "Uhr", use string.Format.

Same for zip + " " + city + " " + street.


This logic is repeated several times: .AddHours(23).AddMinutes(59). Which suggests to me it should be moved to a method.


Avoid names like timesArray and datesArray, especially considering neither of them is an Array, they're List<T>.

Why is a assigned? It isn't used anywhere.

test as a variable name?


Are you certain that GetCoordinates isn't causing endless loops?


There are more issues with your code, but I'd advice you to apply these recommendations and any others you might receive, and post the result as a new question.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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