Open closed principle in a method

Before reviewing micro-design we may focus on overall architecture. You already know that in future you will also need to read an Excel file, it may be a good moment to introduce an abstract base class (or an interface) to abstract this detail from the client:

public abstract class Convertor
{
    public abstract IEnumerable<Company> ToCompanyModel(string path);
}

Note few changes:

• I'm using Convertor as base class name then you will have, for example, CsvConvertor, no need to repeat any of those information in the method name.
• I'm returning IEnumerable<Company> instead of List<Company>. The effective type you're using to store the result is an implementation detail, here I picked the most generic one but you may return IList<Company> as well (what if, for example, you will decide to store Company as an hash-table to detect duplicates?)

How to create the right Convertor instance? I assume you have DI in-place then you may have a factory class which will create the right instance according to file extension (assuming you're sure you will need Excel files). Something similar to:

public interface IConvertorFactory
{
    Convertor Create(string path);
}

If not then simply use DI to get the converter instance without any factory. You now have another problem: testing. Do you want to unit test your converter using physical files? Of course you need to but it's also handy to work with in-memory representations then you may add an overload with StreamReader:

public abstract class Convertor
{
    public abstract IEnumerable<Company> ToCompanyModel(string path);
    public abstract IEnumerable<Company> ToCompanyModel(StreamReader stream);
}

Default implementation may simply open the stream reading line-by-line and you won't need to worry about file size because it won't read everything in memory. Another benefit is that you can check for reader logic without mixing with I/O logic (and this is good to write extensive testing).

ICompanyRepository companyRepository has no reason to be static, you just make your class not thread-safe without any other benefit.

You're not doing any error handling. Things may go wrong and caller will get unknown exception. You'd better handle possible errors and return a single well-known exception (let's say InvalidDataException) with all the required details. Do not forget to document exceptions you may throw. You may even decide to ignore errors in one row and continue processing.

You're splitting each line with row.Split() multiple times, it's a waste: do it once.

To reduce nesting you can use continue and filters (example, again, without error handling):

foreach (string row in csvLines.Where(x => !String.IsNullOrEmpty(x))
{
    var fields = row.Split(',');

    var externalId = fields[0];

    if (companyRepository.CompanyExist(externalId))
        continue;

    // ...
}

However the truth is that you do not need to do CSV parsing by hand. In Microsoft.VisualBasic assembly (if you do not want to use an external library) you already have a well-tested and complete implementation:

using (var parser = new TextFieldParser(new StringReader(stream))) 
{
    parser.SetDelimiters(new string[] { "," });

    // Skip header
    parser.ReadLine();

    while (!parser.EndOfData)
    {
        var fields = parser.ReadFields();

        // Same as before
    }

Note that you can catch MalformedLineException to handle errors and you can specify exactly the expected type of each field (if you want to). Also text fields may be enclosed in quotes, use TextFieldParser.HasFieldsEnclosedInQuotes property to instruct the parser. Blank lines are, by default, skipped and you may even use comments (see TextFieldParser.CommentTokens property).

Very last note: if CSV file is generated by Excel then be aware that it won't always use the comma as delimiter but the current list separator character (see CultureInfo.TextInfo.ListSeparator).

When you actually need the Excel feature, you'll know exactly what you need. Right now, you will only guess and do it wrong.

This might means 2 things:

• You'll have to change it in the future, because it wasn't like you guessed.

• Eventually, you won't need that feature and have made it complex unnecesarily.