PCR analysis on data imported from Excel

Question

I'm writing a program that is going to analyse some data imported from an Excel spreadsheet. It is currently running quite slow (couple seconds on an average machine) and I was wondering whether it could be sped up in any way.

There are three classes containing the data.

This first class contains experimental runs. The data may be excluded in the calculation of sample mean, which is why it has the isIncludedInMean property.

namespace PCR_Analysis
{
    /// <summary>
    /// Contains a single experimental run
    /// </summary>
    public class Experiment
    {
        public double cT;                // cT measured value
        public bool isIncludedInMean;    // included in mean calculations?

        public Experiment(string cTinput)
        {
            // Try render input string as double, otherwise set as NaN
            if (!double.TryParse(cTinput, out cT))
            {
                cT = double.NaN;
                isIncludedInMean = false;     // ignore non-values
            }
            else
            {
                isIncludedInMean = true;
            }
        }

        // Flip isIncludedInMean if cT is a number
        public void flipIsIncludedInMean()
        {
            if (!double.IsNaN(cT))
            {
                isIncludedInMean = !isIncludedInMean;
            }
        }
    }
}

Multiple (usually three) experiments are included in a single sample, which also contains the mean of the experiments, as well as the sample's name:

using System.Collections.Generic;
using System.Linq;

namespace PCR_Analysis.Classes
{
    public class Sample
    {
        public string name { get; }          // sample name
        public double cTmean;                // sample mean
        public List<Experiment> experimentRepeats = new List<Experiment>();

        // Set name when generated
        public Sample(string inputName)
        {
            name = inputName;
            cTmean = double.NaN;
        }

        // Add Experiment to list
        public void AddExperiment(string cTinput)
        {
            Experiment tempExperiment = new Experiment(cTinput);
            experimentRepeats.Add(tempExperiment);
        }

        // Calculate mean
        public void CalculateCTMean()
        {
            // Define list of non-ignored values
            List<Experiment> meanExperiments = experimentRepeats.
                                               Where(x => x.isIncludedInMean).
                                               ToList();

            // Calculate mean if list is non-empty
            if (meanExperiments.Count > 0)
            {
                double sum = meanExperiments.Sum(x => x.cT);
                cTmean = sum / meanExperiments.Count;
            }
        }
    }
}

There will be multiple samples contained within a particular gene experimental run:

using System.Collections.Generic;

namespace PCR_Analysis.Classes
{
    public class Gene
    {
        public string name { get; }                         // Gene name
        public List<Sample> samples = new List<Sample>();   // Sample list

        // Set name when generated
        public Gene(string inputName)
        {
            name = inputName;
        }

        // Add sample to list if it doesn't exist already
        public void AddSample(string sampleName)
        {
            if (!samples.Exists(x => x.name == sampleName))
            {
                Sample tempSample = new Sample(sampleName);
                samples.Add(tempSample);
            }
        }
    }
}

Finally, there is a class to import the data from an excel spreadsheet into these structures. There are some magic numbers at the beginning responsible for the input format, but these will stay constant (the output from the experimental machine is the same all the time).

using System;
using System.Linq;
using Excel = Microsoft.Office.Interop.Excel;
using System.Collections.Generic;

namespace PCR_Analysis.Classes
{
    public class DataImport
    {
        // Global variables
        // Corner cells of the inspected range containing values
        private static string topLeftCell = "B9";
        private static string bottomRightCell = "G1000";
        // Columns for particular data types w.r.t above range
        // i.e. topLeftCell is column 1
        private static int geneColumn = 2;
        private static int sampleColumn = 1;
        private static int cTcolumn = 6;

        /// <summary>
        /// Opens an Excel workbook as read only. Updates links automatically
        /// </summary>
        static public Excel.Workbook OpenWorksheetAsReadOnly(string filePath)
        {
            // Initialise application 
            Excel.Application excelInstance = null;

            // Open workbook as readOnly, update links
            excelInstance = new Excel.Application();
            Excel.Workbook excelWorkbook = excelInstance.Workbooks.
                                           Open(filePath, true, true);

            // Return opened workbook
            return excelWorkbook;
        }

        /// <summary>
        /// Imports data from Excel workbook into a two-dimensional array
        /// </summary>
        static public object[,] ImportExcelIntoArray(string filePath)
        {
            // Initialise and open file
            Excel.Workbook source = null;
            source = OpenWorksheetAsReadOnly(filePath);

            // Load range and convert values to object
            string importRange = topLeftCell + ":" + bottomRightCell;
            Excel.Range sourceRange = source.Sheets[1].Range(importRange);
            object[,] sourceValues = (object[,])sourceRange.Value2;

            // Return
            return sourceValues;
        }

        static public List<Gene> ConvertToGeneList(string filePath)
        {
            // Load file, initialise list
            object[,] input = ImportExcelIntoArray(filePath);
            List<Gene> output = new List<Gene>();

            // Initialise content variables
            string sampleName, geneName, cTstring;

            // Count the number of non-(empty or null) gene cells
            int rowCount = Enumerable.Range(1, input.GetLength(0))  
                            .Count(row => !String.IsNullOrEmpty(
                                Convert.ToString(input[row, 1])));

            // Iterate through all rows
            for (int i = 1; i <= rowCount; i++)
            {
                // Assign variables
                sampleName = Convert.ToString(input[i, sampleColumn]);
                geneName = Convert.ToString(input[i, geneColumn]);
                cTstring = Convert.ToString(input[i, cTcolumn]);

                // Add a gene to list if it doesn't exist and return its index
                if (!output.Exists(g => g.name == geneName))
                {
                    output.Add(new Gene(geneName));
                }
                int geneIndex = output.FindIndex(g => g.name == geneName);

                // Add a sample to gene if it doesn't exist and return its index
                output[geneIndex].AddSample(sampleName);
                int sampleIndex = output[geneIndex].samples
                                    .FindIndex(s => s.name == sampleName);

                // Add cT value to sample
                output[geneIndex].samples[sampleIndex].AddExperiment(cTstring);
            }

            return output;
        }
    }
}

To me it feels like the import could be improved somehow - the files take a while to open and the "check gene, check sample" iterates over the gene list and the sample list within that gene twice for every row analysed (first to see if it exists, then to return its index) - but I don't know how a speed-up/tidying up could be achieved.

Answer 1

First of all, most (if not all) your comments are unnecessary. Your comments should explain how not why (apart from the head comments that resume the signature of the method) you do things. They're there to help your fellow developers to understand what happens, but don't forget that your code is its best documentation. Well written code won't need much comments.

 // Return opened workbook
 return excelWorkbook;

I guess you'll agree with me that it's pretty obvious what return excelWorkbook; does, so no comments should be necessary.

When it comes to coding conventions, you should never have public fields. Encapsulate those in properties. Why? Because, imagine the following scenario (which is not related to your code)

public class Basket
{
    public int capacity;
}

My Basket could become a black hole pretty quickly if I was to set a negative capacity. After all, what stops me?

In your property, you could do :

public class Basket
{
    private int _capacity;
    public int Capacity
    {
        get { return _capacity; }
        set 
        {
            if(value < 0) throw new ArgumentException(nameof(value),"value can't be smaller than 0");
            _capacity = value;
        }
    }
}

Maybe you wonder "Yeah but I'm not dumb enough to change that value by myself". You're probably right, but if someone else was to get in your code. Let's say you win a million dollars and quit your job, the next guy might not have the same wisdom as you and he'll break everything.

Your method and properties should be PascalCased and you should always check your parameters for null in your public methods. You don't want a NullReferenceException to be thrown at a weird moment that you won't be able to debug properly.

Now. All this :

// Define list of non-ignored values
List<Experiment> meanExperiments = experimentRepeats.Where(x => x.isIncludedInMean).ToList();

// Calculate mean if list is non-empty
if (meanExperiments.Count > 0)
{
    double sum = meanExperiments.Sum(x => x.cT);
    cTmean = sum / meanExperiments.Count;
}

could be replaces using the Average Linq method.

ctMean = experimentRepeats.Where(x => x.isIncludedInMean).Average(c => c.cT);

It's not much faster but it's certainly better.

I think if you focus on making your code tidier, it'll be easier for you to find places to speed up your code. Otherwise, you could use tools to monitor your code to see what's slow.