Country code lookup for each line in a CSV file

Question

I'm trying to write a program that has a for loop, inside of which I have lambda expression to get a certain value. The loop has about 20,000 iterations. Including this lambda expression increases the execution time significantly. If I include it, it takes about 20 mins for the loop to finish; without it, it takes just 10 seconds.

Here's what the lambda expression looks like:

code = countries.Find(c => c.ISO2Code == iso2).Country_Code

Same expression using Linq:

code = (from a in countries
        where a.ISO2Code == iso2
         select a.Country_Code).First();

Here, the list countries is cached for the loop starts, so it's not like the query is made to the DB. What's the reason for this time costly expression?

Here's the full code for the loop:

for (int i = 1; i < count - 1; i++)
{
    if (!timer.IsRunning)
        timer.Start();

    var d = line[i];
    var row = d.Replace("\n", "");
    var values = row.Split(',');

    if (values.Length < 3)
    {
        return;
    }


    if (values.Length >= 2)
    {
        if (values[2] != null && values[2] != "")
        {
            string contactno = Utils.RemoveSpecialCharacters(values[2]);


            if (model.NumberFormat == 1)
            {
                Country cntry =
                    countries.Where(x => x.Country_Code == model.CountryCode)
                        .FirstOrDefault();

            contactno = utils.GetValidNumberWithCountryCode(contactno, cntry.ISO2Code);
            }

            int? code = null;
            try
            {
                contactno = "+" + contactno;
                try
                {
                //this is commented
                ph = phoneUtil.parseAndKeepRawInput(contactno, "US");
                if (phoneUtil.isValidNumber(ph))
                {
                    string iso2 = phoneUtil.getRegionCodeForNumber(ph);

                    if (!string.IsNullOrEmpty(iso2))
                    {
                        code = countries.Find(c => c.ISO2Code == iso2).Country_Code;
                        //code = (from a in countries
                        //        where a.ISO2Code == iso2
                        //        select a.Country_Code).First();
                    }
                }
            }
            catch
                {
                    code = null;
                }
            }
            catch
            {
            }

            if (code != null && utils.IsValidNumber(contactno))
            {
                contactno = utils.GetValidNumber(contactno);

                bool match = false;
                foreach (var item in existingList)
                {
                    if (item.ContactNumber.ToString() == contactno)
                    {
                        ExistingnumberCount += 1;
                        match = true;
                        break;
                    }
                }
                if (!match)
                {
                    validNumberCount += 1;
                    Contact_List cl = new Contact_List();
                    cl.Customer_Code = model.Customer_Code;
                    try
                    {
                        string fname = values[0];
                        if (fname.Length > 50)
                        {
                            fname = fname.Substring(0, 50);
                        }
                        cl.First_Name = fname;
                    }
                    catch
                    {
                    }
                    try
                    {
                        string lname = values[1];
                        if (lname.Length > 50)
                        {
                            lname = lname.Substring(0, 50);
                        }
                        cl.Last_Name = lname;
                    }
                    catch
                    {
                    }

                    try
                    {
                        string CF1 = values[3];
                        if (CF1.Length > 50)
                        {
                            CF1 = CF1.Substring(0, 50);
                        }
                        cl.CF1 = CF1;
                    }
                    catch
                    {
                    }
                    try
                    {
                        string CF2 = values[4];
                        if (CF2.Length > 50)
                        {
                            CF2 = CF2.Substring(0, 50);
                        }
                        cl.CF2 = CF2;
                    }
                    catch
                    {
                    }
                    try
                    {
                        string CF3 = values[5];
                        if (CF3.Length > 50)
                        {
                            CF3 = CF3.Substring(0, 50);
                        }
                        cl.CF3 = CF3;
                    }
                    catch
                    {
                    }
                    try
                    {
                        string CF4 = values[6];
                        if (CF4.Length > 50)
                        {
                            CF4 = CF4.Substring(0, 50);
                        }
                        cl.CF4 = CF4;
                    }
                    catch
                    {
                    }

                    cl.CountryCode = code;
                    cl.Contact_No = contactno.Replace("+", "");
                    cl.Is_Active = true;
                    cl.Create_Date = DateTime.Now;
                    cl.Opt_Status_Code = 0;
                    try
                    {
                        string Contact_Email = values[7];
                        if (Contact_Email.Length > 50)
                        {
                            Contact_Email = Contact_Email.Substring(0, 50);
                        }
                        cl.Contact_Email = Contact_Email;
                    }
                    catch
                    {
                    }
                } 
            }
            else
            {
                InvalidnumberCount += 1;
            }
        }
    }

    Debug.WriteLine("Total Execution Time: " + (double)(timer.ElapsedMilliseconds / 1000));
    Debug.WriteLine("No of iterations completed: " + i);
}

Answer 1

Your loop is close to 200 lines, that's waaaay too long. That's long for a full class, let alone one method, let alone part of one method.

---

Why do you use for (int i = 1; i < count - 1; i++) when you don't do anything with i? Why not use foreach(var line in lines)? Oh wait, I see, you've called your collection line -- a bad name, since this doesn't tell me it is a collection of lines.

---

You assign d, a name that conveys absolutely nothing, only to use that to execute var row = d.Replace("\n", "");. Why not do that in one line?

---

Everything inside if (values.Length >= 2) should be moved to a separate method; matter of fact make that everything inside if (values[2] != null && values[2] != ""). Also, don't you know about string.IsNullOrEmpty()?

---

values[0], values[1], values[2], etc. are meaningless. It would be far better to convert each line into a class with properly named properties.

---

Then suddenly we encounter if (model.NumberFormat == 1), yet model is nowhere defined. I assume this comes from outside the for loop? If so, then why is this code located here?

---

Property names should not contain an underscore: Country_Code.

---

Logic like "retrieve contactno", "retrieve code", etc. should all be a method of their own.

---

You are using try...catch waaaay too much. I mean, this is just nuts:

int? code = null;
try
{
    contactno = "+" + contactno;
    try
    {
        //this is commented
        ph = phoneUtil.parseAndKeepRawInput(contactno, "US");
        if (phoneUtil.isValidNumber(ph))
        {
            string iso2 = phoneUtil.getRegionCodeForNumber(ph);

            if (!string.IsNullOrEmpty(iso2))
            {
                code = countries.Find(c => c.ISO2Code == iso2).Country_Code;
                //code = (from a in countries
                //        where a.ISO2Code == iso2
                //        select a.Country_Code).First();
            }
        }
    }
    catch
    {
        code = null;
    }
}
catch
{
}

Do you seriously expect contactno = "+" + contactno; to throw an exception?

---

Why is there a comment saying //this is commented?

---

This:

bool match = false;
foreach (var item in existingList)
{
    if (item.ContactNumber.ToString() == contactno)
    {
        ExistingnumberCount += 1;
        match = true;
        break;
    }
}

can basically be replaced by match = existingList.Any(x => x.ContactNumber.ToString() == contactno);.

---

At that point I'm not even halfway through and I have to give up. You need to seriously refactor this code. Each property you need to retrieve should have its own method, and in 99% of those cases there shouldn't be an empty try...catch block.

WRT your question: consider converting countries to a Dictionary<string, string>, e.g. var countryCodeByIso2 = countries.ToDictionary(x => x.ISO2Code, x => x.Country_Code); and then use TryGetValue(). But please, first refactor your code.

Answer 2

Instead of using linq lookups for the lists you should map your data into dictionaries, which are MUCH faster to access specific instance of your data, memorywise you might take a hit but the mapping is just onetime.

var countries = new List<Country>();
var countriesByCode = countries.ToDictionary(c => c.Country_Code, c => c);

Country cntry;
if (countriesByCode.TryGetValue(countryCody, out cntry))
{
    contactno = utils.GetValidNumberWithCountryCode(contactno, cntry.ISO2Code);
}

Consider this sample on what is the performance difference.

var stringList = Enumerable.Range(0, 50000).Select(s => new Tuple<int, string>(s, s.ToString())).ToList();
var dictionary= stringList.ToDictionary(s => s.Item1, s => s);

var sw = new Stopwatch();
sw.Start();
for (var i = 50000; i > 0; i--)
{
    var t = stringList.FirstOrDefault(s => s.Item1 == i);
    if (t != null)
    {
    }
}
Console.WriteLine(sw.Elapsed);

sw.Restart();
for (var i = 50000; i > 0; i--)
{
    Tuple<int, string> t;
    if (dictionary.TryGetValue(i, out t))
    {
    }
}
Console.WriteLine(sw.Elapsed);

It takes about 20seconds to perform the lookups through the list, but under a millisecond to perform the same with dictionary lookups. Memory is consumed a bit more however.

Answer 3

if (values.Length < 3)
{
    return;
}


if (values.Length >= 2)
{  

How should in the second if the values.Length ever be anything than > 2 ? That second if is superflous and doesn't add anything but a level of intendation to the code and makes it less readable.

Answer 4

Be sure that countries is cached
You could close the DB connection before the loop to test

Move this to before the loop

if (!timer.IsRunning)
    timer.Start();

-

values[2] != null && values[2] != ""

could be

!String.IsNullOrEmpty(values[2])

Duplicate catch

Some indents wrong

How would this ever fail?
You know Length >= 2

try 
{   string fname = values[0];
    if (fname.Length > 50)
    {
       fname = fname.Substring(0, 50);
    }
    cl.First_Name = fname;
 }

Go with

 cl.First_Name = values[0].Length > 50 ? values[0].Substring(0, 50) : values[0];

 if(values.Length >= 3)
 {
     cl.CF2 = values[4].Length > 50 ? values[4].Substring(0, 50) : values[4];
 }

You create a new Contact_List and do not actually use it for anything

Contact_List cl = new Contact_List();

convert to double after you divide adds not value

(double)(timer.ElapsedMilliseconds / 1000)