Some suggestions for improvements

Jun 18, 2009 at 4:29 PM

        public virtual string Serialize()
{
System.Xml.Serialization.XmlSerializer xmlSerializer =
new System.Xml.Serialization.XmlSerializer(this.GetType());

The XmlSerializer should be static and initialized only once in the static ctor for the class. This will improve performance quite a bit.

public virtual bool SaveToFile(string fileName, out System.Exception exception)

That is just too weird a method signature ;).
Why not provide a regular Save(string fileName) which throws, and a TrySave(string fileName, out Exception) ?

Jun 22, 2009 at 9:43 AM
Thank you for your message daniel.
I am currently working on a base class that contains all the methods of serialization.
As you can see below, base class will contains several overloads of Save.
This evolution of Xsd2Code will be available during the month of July in release 2.8.6.0.
French :
Merci pour votre message daniel,
Je travaille actuellement sur la génération d'une classe de base pour héberger toutes les méthodes de sérialisation.
Il y a plusieurs surcharge qui selon le cas lève ou non les execptions.
Cette évolution sera disponible dans le mois de juillet.
Pascal.
 
#region Base entity
    public partial class EntityBase<T>
    {

        /// <summary>
        /// Serializes current EntityBase object into an XML document
        /// </summary>
        // <returns>string XML value</returns>
        public virtual string Serialize()
        {
            System.Xml.Serialization.XmlSerializer xmlSerializer = new System.Xml.Serialization.XmlSerializer(this.GetType());
            System.IO.MemoryStream memoryStream = new System.IO.MemoryStream();
            xmlSerializer.Serialize(memoryStream, this);
            memoryStream.Seek(0, System.IO.SeekOrigin.Begin);
            System.IO.StreamReader streamReader = new System.IO.StreamReader(memoryStream);
            return streamReader.ReadToEnd();
        }

        /// <summary>
        /// Deserializes workflow markup into an EntityBase object
        /// </summary>
        // <param name="xml">string workflow markup to deserialize</param>
        // <param name="obj">Output EntityBase object</param>
        // <param name="exception">output Exception value if deserialize failed</param>
        // <returns>true if this XmlSerializer can deserialize the object; otherwise, false</returns>
        public static bool Deserialize(string xml, out T obj, out System.Exception exception)
        {
            exception = null;
            obj = default(T);
            try
            {
                obj = Deserialize(xml);
                return true;
            }
            catch (System.Exception ex)
            {
                exception = ex;
                return false;
            }
        }

        public static bool Deserialize(string xml, out T obj)
        {
            System.Exception exception = null;
            return Deserialize(xml, out obj, out exception);
        }

        public static T Deserialize(string xml)
        {
            System.IO.StringReader stringReader = new System.IO.StringReader(xml);
            System.Xml.XmlTextReader xmlTextReader = new System.Xml.XmlTextReader(stringReader);
            System.Xml.Serialization.XmlSerializer xmlSerializer = new System.Xml.Serialization.XmlSerializer(typeof(T));
            return ((T)(xmlSerializer.Deserialize(xmlTextReader)));
        }

        /// <summary>
        /// Serializes current EntityBase object into file
        /// </summary>
        // <param name="fileName">full path of outupt xml file</param>
        // <param name="exception">output Exception value if failed</param>
        // <returns>true if can serialize and save into file; otherwise, false</returns>
        public virtual bool SaveToFile(string fileName, out System.Exception exception)
        {
            exception = null;
            try
            {
                SaveToFile(fileName);
                return true;
            }
            catch (System.Exception e)
            {
                exception = e;
                return false;
            }
        }

        public virtual void SaveToFile(string fileName)
        {
            string xmlString = Serialize();
            System.IO.FileInfo xmlFile = new System.IO.FileInfo(fileName);
            System.IO.StreamWriter streamWriter = xmlFile.CreateText();
            streamWriter.WriteLine(xmlString);
            streamWriter.Close();
        }

        /// <summary>
        /// Deserializes workflow markup from file into an EntityBase object
        /// </summary>
        // <param name="xml">string workflow markup to deserialize</param>
        // <param name="obj">Output EntityBase object</param>
        // <param name="exception">output Exception value if deserialize failed</param>
        // <returns>true if this XmlSerializer can deserialize the object; otherwise, false</returns>
        public static bool LoadFromFile(string fileName, out T obj, out System.Exception exception)
        {
            exception = null;
            obj = default(T);
            try
            {
                System.IO.FileStream file = new System.IO.FileStream(fileName, FileMode.Open, FileAccess.Read);
                System.IO.StreamReader sr = new System.IO.StreamReader(file);
                string xmlString = sr.ReadToEnd();
                sr.Close();
                file.Close();
                return Deserialize(xmlString, out obj, out exception);
            }
            catch (System.Exception ex)
            {
                exception = ex;
                return false;
            }
        }
    }
    #endregion
Jun 22, 2009 at 11:24 AM

Not sure I like having to inherit from a base class just to get members that are not specific to the type at hand... maybe making these extension methods would be enough?

Anyway, there's still a couple improvements that I think should definitely be implemented:

- Make XmlSerializer static:

 public partial class EntityBase<T>
{
static readonly XmlSerializer Serializer = new XmlSerializer(typeof(T));

then whenever you need to serialize/deserialize, just use that static serializer.

- Leverage using() construct:

*everywhere* you use a disposable object (i.e. a FileStream, a StreamReader, etc.), wrap it in a using(..), such as:

public static bool LoadFromFile(string fileName, out T obj, out System.Exception exception)
{
exception = null;
obj = default(T);
try
{
using (System.IO.FileStream file = new System.IO.FileStream(fileName, FileMode.Open, FileAccess.Read))
using (System.IO.StreamReader sr = new System.IO.StreamReader(file))
{
string xmlString = sr.ReadToEnd();
return Deserialize(xmlString, out obj, out exception);
}
}
catch (System.Exception ex)
{
exception = ex;
return false;
}
}

Note how now you don't need to explicitly call .Close() on either object, and you have ensured they are properly closed because of the using(..) construct. In your existing code, if an exception happens in the line with string xmlString = sr.ReadToEnd(), you'd never get to close the file stream :S.

Also, in this method you're not taking into account the encoding of the input XML. You must read Xml using an XmlReader (which you should create by calling XmlReader.Create, rather then new XmlTextReader which is obsolete), as it's the only one that knows about and processes the <? xml ?> directive that contains encoding information. Otherwise, your code will fail for certain documents.

I'd make it so that my main implementation of Deserialize works with an XmlReader over the input stream/file, *always*.

Thanks and keep up the good work!
Jun 22, 2009 at 1:59 PM

Daniel thank you for all these improvements.

First, inheritance is done automatically by Xsd2Code. You don't have to do it your self. You can have the methods inside the class or in a base class. This will be an option.
I'am agree with you to make XmlSerializer static. (task 7914).
About using, i also agree with you on this point. (task 7915).

But to my knowledge, using statment is not supported by CodeDom.

I can probably do it manually with CodeSnippetStatement.
But I don't think that is the best way because the using statement is not supported across languages.

In any case, i can use try...finally to do it.

        public static bool LoadFromFile(string fileName, out T obj, out System.Exception exception)
        {
            exception = null;
            obj = default(T);
            FileStream file = null;
            StreamReader sr = null;
            try
            {
                file = new System.IO.FileStream(fileName, FileMode.Open, FileAccess.Read);
                sr = new System.IO.StreamReader(file);
                string xmlString = sr.ReadToEnd();
                return Deserialize(xmlString, out obj, out exception);
            }
            catch (System.Exception ex)
            {
                exception = ex;
                return false;
            }
            finally
            {
                if (file != null)
                    file.Dispose();
                if (sr != null)
                    sr.Dispose();
            }
        }


I'll change XmlTextReader to XmlReader. (task 7916).
Thank's
Pascal.

Jun 22, 2009 at 2:24 PM

The base class "issue" is more related to the need to be "POCO". I may need to inherit from my own business object base class (or for example, entity framework base class), and this would prevent that.

I'm glad the other suggestions were useful :)