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.