patterncsharpMinor
Xml serialize to object
Viewed 0 times
serializeobjectxml
Problem
I have to read some data from xml file and then need to store the converted json in the database. The class
I have some concerns with the function
Also please let me know if there is any efficient solutions than this logic.
DtoProcedureXml mapped with xml attributes.public class ProcedureManager
{
#region Declaration
private DtoProcedureXml procedure;
private ProcedureDataManager procedureDataManager;
#endregion
public ProcedureManager()
{
this.procedureDataManager = new ProcedureDataManager();
}
public bool SaveProcedureData(long procedureID)
{
bool isSuccess = false;
try
{
SetProcedureXmlData();
isSuccess = SaveProcedureSummary(procedureID);
}
catch (Exception ex)
{
CommonLogger.LogException("Failed to save procedure data");
}
return isSuccess;
}
private bool SaveProcedureSummary(long procedureID)
{
//saving the data to db
return procedureDataManager.Save(new DtoProcedure { ProcedureID = procedureID, JsonData = Newtonsoft.Json.JsonConvert.SerializeObject(this.procedure), CreatedOn = DateTime.Now });
}
private void SetProcedureXmlData()
{
this.procedure = new DtoProcedureXml();
XmlSerializer serializer = new XmlSerializer(typeof(DtoProcedureXml));
using (XmlTextReader reader = new XmlTextReader(FileShareUrlBuilder.GetPath(Constants.Structures.PathType.XMLFILEPATH)))
{
object obj = serializer.Deserialize(reader);
this.procedure = (DtoProcedureXml)obj;
reader.Close();
}
}
}I have some concerns with the function
SetSReportXmlData() When I was debugging the XmlSerializer code takes few milliseconds, is it right to move that code in the using section?Also please let me know if there is any efficient solutions than this logic.
Solution
I would suggest not to recreate the instance of
see also: XmlSerializer class may result in a memory leak and poor performance
XmlSerializer each time, because it creates an assembly in memory that is not reused / unloaded. To avoid that, just create the instance as static field of the class or use something like a XmlSerializerCache.see also: XmlSerializer class may result in a memory leak and poor performance
- The value of
procedurecould be returned bySetProcedureXmlDataand passed toSaveProcedureSummary. That makes it more clear the the methods have to be called in that order.
- The instance variable
procedureDataManagercould be readonly
- You could drop the
isSuccessvariable if you return the result ofSaveProcedureSummaryor false in the catch block.
- It would be more readable if the line in
SaveProcedureSummaryis splitted to 2 or 3 lines.
- The using is OK, but ther is no need to close the reader because it will be closed when the reader is disposed by the using.
Context
StackExchange Code Review Q#134722, answer score: 5
Revisions (0)
No revisions yet.