Empty Interface usage - Is this a code smell?

Question

I've recently made a set of interfaces/classes to work with converting a spreadsheet into an object but I utilise an empty interface in my design:

I have my interface which defines what a spreadsheet object should have:

public interface IParsedSpreadsheet<TEntity> where TEntity: IParsedRow
{
    int Columns { get; set; }
    int Pages { get; set; }
    Dictionary<string, int> Map { get; set; }
    List<string> ColumnHeaders { get; set; }
    List<TEntity> RowList { get; set; }
    List<TEntity> ParseSheet(IFileStorage storage);
    List<String> ObtainColumnHeaders(IFileStorage storage);
}

As you can see it take a type parameter which must implement IParsedRow which is my empty interface:

public interface IParsedRow
{
    //Marker Interface
}

Here is an abstract class declaration that implements the interface:

public abstract class AbstractParsedSpreadsheet<TEntity> : IParsedSpreadsheet<TEntity>, IEnumerable<TEntity> where TEntity : IParsedRow, new()

One of the important parts of this class is the Map. This has a list of column headers that should appear in a spreadsheet and also the index of the column they appear in.

Any concrete implementation of IParsedRow will have a number of properties each of which must be named after a column header in the spreadsheet it will represent. I use the Map along with reflection to determine that the spreadsheet headers match up to the TEntity's properties exactly to let me know that the uploaded spreadsheet is valid for the map it is being put against.

Here is my abstract class in full (note I use NPOI as a wrapper to easily parse a spreadsheet. NPOI is an open source spreadsheet framework for .NET):

    /// <summary>
    /// Abstract, generic class that only accepts a type parameter which implements IParsedRow for use in its internal row collection.
    /// </summary>
    /// <typeparam name="TEntity"></typeparam>
    public abstract class AbstractParsedSpreadsheet<TEntity> : IParsedSpreadsheet<TEntity>, IEnumerable<TEntity> where TEntity : IParsedRow, new()
    {
        public int Columns { get; set; }
        public int Pages { get; set; }
        public Dictionary<string, int> Map { get; set; }
        public List<string> ColumnHeaders { get; set; }
        public List<TEntity> RowList { get; set; }

        /// <summary>
        /// Single parametised constructor which will perform all work needed to make an object from a spreadsheet.
        /// Used on the front end this might have a dropdown list of all "maps" or spreadsheet types you can upload.
        /// and then the act of uploading will create a storage item. If the spreadsheet in the storage item doesnt match
        /// up to the map then an error will be thrown.
        /// </summary>
        /// <param name="validationMap">this should hold the names of the columns in the spreadsheet (and also the TEntities properties) and the column index in which the column name resides in</param>
        /// <param name="storage">this object has a property which refers to a file location and is used by NPOI to load up a spreadsheet for checking and parsing.</param>
        public AbstractParsedSpreadsheet(Dictionary<string,int> validationMap, IFileStorage storage)
        {
            this.Map = validationMap;

            //Check validation map against properties of TEntity
            if (!this.CheckMapMatchesRowType())
            {
                throw new InvalidDataException("Invalid Map/Type parameter used");
            }

            //Obtain column headers from spreadsheet
            this.ColumnHeaders = ObtainColumnHeaders(storage);

            //Check validationMap against column headers
            if (!CheckHeadersAgainstMap())
            {
                throw new InvalidDataException("Invalid Spreadsheet/Map used");
            }

            //Parse spreadsheet into RowList if all of the above pass.
            this.RowList = ParseSheet(storage);
        }

        /// <summary>
        /// This method takes in an IFileStorage implementation and uses it to locate and open a spreadsheet.
        /// It then reads formthe spreadsheet, calling another function to create objects of type TEntity
        /// and adds them into a list which belongs to this class.
        /// </summary>
        /// <param name="storage"></param>
        /// <returns></returns>
        public List<TEntity> ParseSheet(IFileStorage storage)
        {
            List<TEntity> ListOfRows = new List<TEntity>();

            HSSFWorkbook hssfbook;

            using (FileStream file = new FileStream(storage.StorageLocation, FileMode.Open, FileAccess.Read))
            {
                hssfbook = new HSSFWorkbook(file);
            }

            ISheet sheet = hssfbook.GetSheetAt(0);

            foreach (IRow row in sheet)
            {
                if (row.RowNum == 0)
                {
                    continue;
                }
                else
                {

                    ListOfRows.Add(CreateEntityFromRow(row));
                }
            }

            return ListOfRows;
        }

        /// <summary>
        /// Bit of a complicated one - Accepts an IRow implementing object (those used by the NPOI spreadsheet classes)
        /// looks up the column index of each cell in a row and maps it using the local Map variable (dictionary of string to int)
        /// to a string value. This value can then be used to dynamically obtain a property name from TEntity using .NET Reflection.
        /// The value of the current cell is then set to that property on TEntity before being continuing to the next cell.
        /// After the entire object is populated it returns it.
        /// </summary>
        /// <param name="row"></param>
        /// <returns></returns>
        public TEntity CreateEntityFromRow(IRow row)
        {
            TEntity retVal = new TEntity();
            Type entity = typeof(TEntity);
            int propertyCount = entity.GetProperties().Count();

            foreach (ICell c in row)
            {
                //Looks up the column index of the current cell and Maps it to the corresponding value in the Map dictionary to 
                //obtain the correct property name in TEntity that this value needs to be set for.
                string columnName = this.Map.Where(d => d.Value == c.ColumnIndex).Select(e => e.Key).First();

                switch (c.CellType)
                {
                    case CellType.STRING:
                        retVal.GetType().GetProperty(columnName).SetValue(retVal, c.StringCellValue.ToString(), null);
                        break;
                    case CellType.NUMERIC:
                        retVal.GetType().GetProperty(columnName).SetValue(retVal, c.NumericCellValue, null);
                        break;
                    case CellType.BOOLEAN:
                        retVal.GetType().GetProperty(columnName).SetValue(retVal, c.BooleanCellValue, null);
                        break;
                    case CellType.BLANK:
                    case CellType.ERROR:
                    case CellType.FORMULA:
                    case CellType.Unknown:
                    default:
                        break;
                }
            }

            return retVal;

        }
        /// <summary>
        /// Looks up the generic parameter for this class, instatiates it and checks that its properties match the map.
        /// It then checks to ensure that the map contains the correct number of entries for the number of properties on
        /// the generic type.
        /// </summary>
        /// <returns></returns>
        public bool CheckMapMatchesRowType()
        {
            Type entity = typeof(TEntity);
            var properties = entity.GetProperties();

            if (properties.Count() != Map.Count)
            {
                return false;
            }

            foreach (var i in properties)
            {
                if (!Map.Keys.Contains(i.Name.ToLower())){
                    return false;
                }
            }

            return true;
        }

        /// <summary>
        /// Gets the top row of any spreadsheet (which is normally where the headers are)
        /// </summary>
        /// <param name="storage"></param>
        /// <returns></returns>
        public virtual List<string> ObtainColumnHeaders(IFileStorage storage)
        {
            HSSFWorkbook hssfbook;
            List<string> ColumnHeaders = new List<string>();

            using (FileStream file = new FileStream(storage.StorageLocation, FileMode.Open, FileAccess.Read))
            {
                hssfbook = new HSSFWorkbook(file);
            }

            ISheet sheet = hssfbook.GetSheetAt(0);
            IRow row = sheet.GetRow(0);

            foreach (ICell c in row)
            {
                switch (c.CellType)
                {
                    case CellType.STRING:
                        ColumnHeaders.Add(c.StringCellValue.ToString().Replace(" ", string.Empty));
                        break;
                    case CellType.NUMERIC:
                    case CellType.BOOLEAN:
                    case CellType.BLANK:
                    case CellType.ERROR:
                    case CellType.FORMULA:
                    case CellType.Unknown:
                    default:
                        break;
                }

            }

            return ColumnHeaders;
        }

        /// <summary>
        /// Checks that the headers obtained from the spreadsheet passed in are valid against the map that has been passed in
        /// also checks that the count of both of them matches.
        /// 
        /// </summary>
        /// <returns></returns>
        public virtual bool CheckHeadersAgainstMap(){
            if (ColumnHeaders.Count != this.Map.Values.Count)
            {
                return false;
            }
            foreach (string i in this.ColumnHeaders)
            {
                if (!this.Map.Keys.Contains(i.ToLower()))
                {
                    return false;
                }
            }
            return true;
        }

        /// <summary>
        /// Make the RowList propert of the class it's enumerable.
        /// </summary>
        /// <returns></returns>
        public IEnumerator<TEntity> GetEnumerator()
        {
            foreach (TEntity t in this.RowList)
            {
                if (t == null)
                {
                    break;
                }

                yield return t;
            }
        }

        [ExcludeFromCodeCoverage]
        IEnumerator IEnumerable.GetEnumerator()
        {
            return this.GetEnumerator();
        }
    }

How can I improve my design? Is my use of an empty interface in this case a code smell? Should I just remove it and allow <TEntity> to be of any type as the mapping functions will catch it anyway and throw an exception?

Answer 1

An empty interface is a code smell. In C# you can use attributes to mark a class instead of making it implement an empty interface.

As I suggested in my comment, you can use reflection to build your Map instead of receiving it as input, and failing on discrepancy. With the power of attributes, you can add functionality by marking which properties you want to map, which to ignore, maybe add metadata (like the column index):

[AttributeUsage(AttributeTargets.Class)]
public class ParsedRowAttribute : Attribute {
}

[AttributeUsage(AttributeTargets.Property)]
public class ParsedCellAttribute : Attribute {
    public ParsedCellAttribute(int columnIndex) {
        this.ColumnIndex = columnIndex;
    }

    public int ColumnIndex { get; set; }
}

If you want to add the column index from an outside source, you can use it as overrides to the code, so you can still count on the class to tell you which properties it externalizes, and validate the override mapping to see there are no typos there.

Answer 2

It may appear to some as code smell, but it's a practice in use in the .net framework defined as 'marker interfaces'.

IReadOnlySessionState is one of these and as per the documentation:

Specifies that the target HTTP handler requires only read access to session-state values. This is a marker interface and has no methods.

So, to answer your question, is this a code smell? perhaps it depends who you ask. In my opinion, if it's good enough 'pattern' for the .net framework, it's good enough for me.

http://msdn.microsoft.com/en-us/library/system.web.sessionstate.ireadonlysessionstate(v=vs.110).aspx