CsvDataReader and ExcelDataReader should share an interface

[SteveLemire]

First, thank you for your awesome library!! I will participate for sure to enhance it because I love it!

I have to implement a data import and I need to support all the main Excel file types: csv, xls, xlsx.
I have a FileManager.ReadExcel that encapsulate all the IFormFile stream manipulation, validations. etc.
I also use the DbDataReader.GetRecord so both CsvDataReader and ExcelDataReader share the same code with the same options.

            DbDataReader dataReader = null;
            using StreamReader streamReader = new StreamReader(formFile.OpenReadStream());

            if (fileRequest.FileExtension == ".csv")
            {                
                dataReader = await CsvDataReader.CreateAsync(streamReader, csvOptions);
            }
            else if (fileRequest.FileExtension == ".xls")
            {
                dataReader = ExcelDataReader.Create(streamReader.BaseStream, ExcelWorkbookType.Excel, excelOptions);
            }
            else if (fileRequest.FileExtension == ".xlsx")
            {
                dataReader = ExcelDataReader.Create(streamReader.BaseStream, ExcelWorkbookType.ExcelXml, excelOptions);
            }

My problem is that I need to get access to the RowNumber and RowFieldCount for validations and these are not accessible through DbDataReader (or the interface IDataReader). I have to cast it in a method like GetRowFieldCount(IDataReader)

        private int GetRowFieldCount(IDataReader dataReader)
        {
            if(dataReader is CsvDataReader)
            {
                return ((CsvDataReader)dataReader).RowFieldCount;
            }
                        
            if(dataReader is ExcelDataReader)
            {
                return ((ExcelDataReader)dataReader).RowFieldCount;
            }

            throw new ArgumentOutOfRangeException("The excel or csv data reader is not supported.");
        }

I know this can affect performances, create overhead, etc.

I think it could be a nice improvement to have an interface for these properties (and probably more other properties/methods) that both readers shared:

        public int RowNumber { get; }
        public int RowFieldCount { get; }

Instead of doing a fix just for myself, I would like to share it with you and your community.
I will probably fork your code and create a PR but I would like to have your thought about this before doing it!

Thanks!

[SteveLemire]

I end up with this internal class:

    internal class DataReaderRawInformation
    {
        public DataReaderRawInformation(IDataReader dataReader)
        {
            if (dataReader is CsvDataReader)
            {
                var reader = (CsvDataReader)dataReader;
                this.RowFieldCount = reader.RowFieldCount;
                this.RowNumber = reader.RowNumber;
            }

            else if (dataReader is ExcelDataReader)
            {
                var reader = (ExcelDataReader)dataReader;
                this.RowFieldCount = reader.RowFieldCount;
                this.RowNumber = reader.RowNumber;
            }
            else
            {
                throw new ArgumentOutOfRangeException("The excel or csv data reader is not supported.");
            }
        }

        public int RowFieldCount { get; set; }
        public int RowNumber { get; set; }
    }

[MarkPflug]

Hi Steve, thanks for the enthusiasm for my libraries. I'm always glad when people find them useful.

I totally understand your desire here. I tried to make the shape of the Excel and CSV libraries to be as similar as possible to make it easy for users to move between them. The problem is that in order for them to share an interface, that interface needs to be defined somewhere. I can't define that in either library without one taking a dependency on the other. I don't want to force that on users of either library. The other option would be to define a third assembly that defines just the common interface (Sylvan.Data.Abstractions, for example), but I'm reluctant to do that too. If only my libraries are implementing that interface, then the value is diminished. Ideally, the base DbDataReader would expose these properties, but I don't think I'd be able to convince Microsoft of that.

Having said that, I might offer an alternative design that would allow you to accomplish this more cleanly. You can adapt the two implementations to an interface that you can define yourself. This could also allow you to adapt other DbDataReaders that might also expose similar properties with different names.

See this gist, as an example:
https://gist.github.com/MarkPflug/9683e882584a72a616d3b513a1aae7f1

You would then be free to expand the ITabularData interface to include whatever properties make sense. You could also add other implementations for DbDataReaders other than mine.

Hope this helps!

[SteveLemire]

Thanks for your answers, I understand your point but I think having a core library that share interfaces and other features is totally fine.

[SteveLemire]

To continue on the same discussion around validation, when I validate the exact same file saved in different format (csv and excel), the dataReader.RowNumber is different! The excel reader have the right RowNumber but the csv reader seams to not count the header row. I would preferer that it use the real file row number. Is this behavior is expected?

PS: I tried true/false with the CsvDataReaderOptions.HasHeader but that change nothing.

UPDATE:
I found in your code in the CsvDataRead.RowNumber and it seems to be the expected behavior!
I would like to understand why because I will have to increase the value by one to correspond to my needs to have the "real row number" if open the CSV in excel or notepad++.

Thanks!

/// <summary> Gets the current 1-based row number of the data reader.</summary>
public int RowNumber => rowNumber;

[MarkPflug]

The CsvDataReader behavior is intended. With CSV data the line number of the text file and the row number won't necessarily agree. In addition to a header row, a single record can span multiple lines, there can be comment (#) lines, and also my library supports an optionf or allowing multiple result sets within one file. For all of these reasons I chose to have the RowNumber correspond to the record number within the result set instead of the line number of the original file.

ExcelDataReader is a bit different, because in Excel the line number and the record number are always 1 to 1, with the exception of the header row. That's why I chose to have the RowNumber correspond to the line number as reported in Excel.

If anything, I would likely change the Excel behavior to skip the header row when counting lines. This would make the two readers agree, but it sounds like it doesn't align with your expectations.

[SteveLemire]

Thanks for the explanation!!
For my need I just added 1 to the csvDataReader.RowNumber and that's it!

[SteveLemire]

Here is my Sylvan wrapper around CsvDataReader and ExcelDataReader:

   public static class SylvanExcelOptions
    {
        public static CsvDataReaderOptions csvOptions = new CsvDataReaderOptions()
        {
            DateFormat = "yyyy-MM-dd",
            Culture = System.Globalization.CultureInfo.InvariantCulture,
            HasHeaders = true,
            HeaderComparer = StringComparer.InvariantCultureIgnoreCase,

            // allow empty data value to set TEntity property to null
            // this is also required to CSV work like Excel
            Schema = CsvSchema.Nullable,
            StringFactory = new StringPool().GetString,
        };

        public static ExcelDataReaderOptions excelOptions = new ExcelDataReaderOptions()
        {
            GetErrorAsNull = false,
        };

        public static DataBinderOptions dataBinderOptions = new DataBinderOptions()
        {
            // AllColumns is required to validate header column name map to an existing entity property.
            BindingMode = DataBindingMode.AllColumns,

            // Convert field to TEntity property type must be false
            // do not set to true or because it have a huge impact on the binder performances
            InferColumnTypeFromMember = false,
        };
    }

    /// <summary>
    /// A wrapper to encapsulate shared properties from CsvDataReader and ExcelDataReader.
    /// </summary>
    public class SylvanExcelDataReader
    {
        public static async Task<SylvanExcelDataReader> CreateAsync(IFormFile formFile)
        {
            DbDataReader dataReader;
            var fileExtension = Path.GetExtension(formFile.FileName)?.ToLower();
            if (fileExtension == ".csv")
            {
                dataReader = await CsvDataReader.CreateAsync(new StreamReader(formFile.OpenReadStream()), SylvanExcelOptions.csvOptions);
            }
            else if (fileExtension == ".xls")
            {
                dataReader = ExcelDataReader.Create(formFile.OpenReadStream(), ExcelWorkbookType.Excel, SylvanExcelOptions.excelOptions);
            }
            else if (fileExtension == ".xlsx")
            {
                dataReader = ExcelDataReader.Create(formFile.OpenReadStream(), ExcelWorkbookType.ExcelXml, SylvanExcelOptions.excelOptions);
            }
            else
            {
                // This should never happend because the file extension is already validated
                throw new ArgumentOutOfRangeException(nameof(formFile));
            }

            return new SylvanExcelDataReader(dataReader);
        }

        public static string GetExcelFormulaValue(ExcelErrorCode errorCode)
        {
            switch (errorCode)
            {
                case ExcelErrorCode.Null:
                    return "#NULL!";
                case ExcelErrorCode.DivideByZero:
                    return "#DIV/0!";
                case ExcelErrorCode.Value:
                    return "#VALUE!";
                case ExcelErrorCode.Reference:
                    return "#REF!";
                case ExcelErrorCode.Name:
                    return "#NAME?";
                case ExcelErrorCode.Number:
                    return "Bad range number";
                case ExcelErrorCode.NotAvailable:
                    return "#N/A";
                default:
                    return "Unkown error";
            }
        }

        private bool isCsvDataReader;
        private CsvDataReader csvReader;
        private ExcelDataReader excelReader;

        public SylvanExcelDataReader(DbDataReader dataReader)
        {
            this.isCsvDataReader = (dataReader is CsvDataReader);
            if (isCsvDataReader)
            {
                this.csvReader = dataReader as CsvDataReader;
            }
            else
            {
                this.excelReader = dataReader as ExcelDataReader;
            }

            this.ExcelColumns = dataReader.GetColumnSchema().ToDictionary(key => key.ColumnName, value => value);
            this.FieldCount = this.ExcelColumns.Count;
        }

        public DbDataReader DataReader
        {
            get
            {
                if (this.isCsvDataReader)
                    return this.csvReader;

                return this.excelReader;
            }
        }

        public Dictionary<string, DbColumn> ExcelColumns { get; private set; }

        public int FieldCount { get; }

        public int RowFieldCount
        {
            get
            {
                if (this.isCsvDataReader)
                    return this.csvReader.RowFieldCount;

                var rowFieldCount = new int[] {
                    this.excelReader.RowFieldCount,
                    this.excelReader.VisibleFieldCount
                };

                return rowFieldCount.Max();
            }
        }

        public int RowNumber
        {
            get
            {
                // CsvDataReader.RowNumber is 1-based so I have to increase it by 1 to get the real file row number
                // https://github.com/MarkPflug/Sylvan/discussions/122#discussioncomment-2531787
                if (this.isCsvDataReader)
                    return this.csvReader.RowNumber + 1;

                return this.excelReader.RowNumber;
            }
        }

        public object GetCellData(int columnIndex)
        {
            if (this.isCsvDataReader)
            {
                return this.csvReader[columnIndex];
            }

            var cellType = this.excelReader.GetExcelDataType(columnIndex);
            switch (cellType)
            {
                case ExcelDataType.Numeric:
                case ExcelDataType.DateTime:
                case ExcelDataType.String:
                case ExcelDataType.Boolean:
                    return this.excelReader[columnIndex];
                case ExcelDataType.Error:
                    return GetExcelFormulaValue(this.excelReader.GetFormulaError(columnIndex));
                case ExcelDataType.Null:
                default:
                    return null;
            }
        }


        public void Close()
        {
            if (this.csvReader != null)
            {
                this.csvReader.Close();
                this.csvReader = null;
            }

            if (this.excelReader!= null)
            {
                this.excelReader.Close();
                this.excelReader = null;
            }
        }
    }