Excel to serializable object

Question

I've got an Excel file from which I have to read out data to an object to serialize. So far I came up with this solution, and I'm curious if there are any clearer solutions.

public myClassFromXsd excelToMyClassFromXsd(String excelFile)
            {
                Excel.Application excelApp = new Excel.Application();            
                Excel.Workbook workBook = excelApp.Workbooks.Open(excelFile);
                Excel.Worksheet workSheet = (Excel.Worksheet)workBook.Sheets[1];

                myClassFromXsd spec = new myClassFromXsd();
                UserHeader user = new UserHeader();
                ResultHeader header = new ResultHeader();
                anotherClassFromXsd specOp = new anotherClassFromXsd();
                List<anotherClassFromXsd> specOpList = new List<anotherClassFromXsd>();
                thisIsAClassToo tcd = new thisIsAClassToo();            

                excelApp.Visible = false;            

                tcd.orderNumber = Convert.ToString(workSheet.get_Range("B3").Value); 
                switch (((String)workSheet.get_Range("B4").Value).ToUpper())
                {
                    case "I":
                        tcd.typeOfThis = EnumFromXsd.I;
                        break;
                    case "E":
                        tcd.typeOfThis = EnumFromXsd.E;
                        break;
                    case "D":
                        tcd.typeOfThis = EnumFromXsd.D;
                        break;
                    default:
                        break;
                }
                switch (((String)workSheet.get_Range("B5").Value).ToUpper())
                {
                    case "I":
                        tcd.othersCanAlter = true;
                        break;
                    case "N":
                        tcd.othersCanAlter = false;
                        break;                
                    default:
                        break;
                }
                tcd.oneProperty = Convert.ToString(workSheet.get_Range("B6").Value);
                tcd.onePropertyText = Convert.ToString(workSheet.get_Range("B7").Value);
                tcd.Name = Convert.ToString(workSheet.get_Range("B10").Value);
                tcd.vatNum = Convert.ToString(workSheet.get_Range("B11").Value);
                tcd.city = Convert.ToString(workSheet.get_Range("B12").Value);                  


                specOp.index = 1;
                specOp.operation = Operation.create;
                specOp.Item = tcd;
                specOpList.Add(specOp);    

                header.requestId = Guid.NewGuid().ToString(); 
                header.timestamp = DateTime.Now.ToString("yyyy-mm-ddTH:mm:sszzz");              


                user.user = "TestUser";
                user.passwordHash = "BA32521232AED23798735C78739273GDHS238238723";

                spec.header = header;
                spec.user = user;
                spec.specOps = specOpList;

                excelApp.Quit();
                return spec;
            }       

And the serializer and deserializer methods:

public void serializeXml(myClassFromXsd mcfx)
        {
            using (StreamWriter writer = new StreamWriter("e:\\test.xml", false))
            {

                XmlSerializer serializer = new XmlSerializer(typeof(myClassFromXsd));
                serializer.Serialize(writer, mcfx);
            }
        }

        public myClassFromXsd deserializeXml(String xmlFile)
        {
            myClassFromXsd mcfx = new myClassFromXsd();
            XmlSerializer serializer = new XmlSerializer(typeof(myClassFromXsd));
            StreamReader reader = new StreamReader(xmlFile);
            mcfx = (myClassFromXsd)serializer.Deserialize(reader);
            reader.Close();
            return mcfx;
        }

Answer 1

The indendation looks off - assuming it's not a paste glitch with tabs vs spaces, the scope-opening brace should line up with the method's signature:

public myClassFromXsd excelToMyClassFromXsd(String excelFile)
            {

Like this:

public myClassFromXsd excelToMyClassFromXsd(String excelFile)
{

Also, C# type names should be PascalCase, as well as any public members:

• myClassFromXsd should be MyClassFromXsd
• serializeXml should be SerializeXml
• deserializeXml should be DeserializeXml

The get_Xxxx methods in the COM Interop interface aren't following this convention, but they're COM Interop methods, with their own conventions - they're not an example to follow for typical C# code.

I like that you're wrapping your StreamWriter in a using block. I don't understand why you're not doing the same with the StreamReader in the deserializeXml method - the reader.Close() call could then be removed:

    public myClassFromXsd deserializeXml(String xmlFile)
    {
        myClassFromXsd mcfx = new myClassFromXsd();
        XmlSerializer serializer = new XmlSerializer(typeof(myClassFromXsd));
        using (StreamReader reader = new StreamReader(xmlFile))
        {
            mcfx = (myClassFromXsd)serializer.Deserialize(reader);
        }
        return mcfx;
    }

I would give mcfx a meaningful name: as it stands it looks like it's just a shortened version of the return type's name - I like having a result for these kinds of things. Identifiers should have a pronounceable name that carries their meaning.

Answer 2

One thing to note is that anytime you're creating an instance of Excel, you need to wrap that code in a try...catch...finally. You take in a string file path, but what if it's not a valid file? Then this line

Excel.Workbook workBook = excelApp.Workbooks.Open(excelFile);

will throw an error and you're left with an invisible instance of Excel left hanging.

---

All of these variable types declarations could probably be replaced with the var keyword. I know the last one could.

Excel.Application excelApp = new Excel.Application();            
Excel.Workbook workBook = excelApp.Workbooks.Open(excelFile);
Excel.Worksheet workSheet = (Excel.Worksheet)workBook.Sheets[1];

---

myClassFromXsd tells me almost nothing about what that class is. It's enough to guess that's it's serialized from an xsd file, but that's about it. Even then, it's a name that tells me about implementation details instead of what the object represents.

---

Do yourself a favor and generalize your Serialize method a little bit. It should be generic and take in a file path as an argument.

public void serializeXml<T>(T toSerialize, string filePath)
{
    using (StreamWriter writer = new StreamWriter(filePath, false))
    {
        XmlSerializer serializer = new XmlSerializer(typeof(T));
        serializer.Serialize(writer, toSerialize);
    }
}

And then you could easily reuse that method serializing any object you ever may need to.