11
\$\begingroup\$

I am new to c#, and rather new to design-patterns. I want to create a simple and a most elegant solution for loading and validating a xml file and later on extracting its fields. I've started with a generic IData interface, that gets implemented by a XmlFileData class, but I feel that there is much more to be improved upon, but I lack the knowledge to do it (from design, to naming variables and methods and so on).

So please, have no mercy !!

public interface IData
{
     String GetDataString();
}

class XMLValidator
    {
        private StringBuilder verificationErrorsList = new StringBuilder();
        public string ValidationSchema { get; set; }
        public String ValidationErrors
        {
            get { return verificationErrorsList.ToString(); }
        }
        public bool HasErrors
        {
            get {return (verificationErrorsList.Length > 0);}
        }

        public void Validate(IData xmlData)
        {
            try
            {
                XmlWriterSettings ws = new XmlWriterSettings();
                ws.Indent = true;

                XmlReaderSettings rs = new XmlReaderSettings();
                rs.ValidationType = ValidationType.Schema;
                rs.ValidationEventHandler += new System.Xml.Schema.ValidationEventHandler(rs_ValidationEventHandler);
                rs.Schemas.Add(null, ValidationSchema);
                rs.CloseInput = true;

                rs.ValidationFlags = XmlSchemaValidationFlags.ReportValidationWarnings |  XmlSchemaValidationFlags.ProcessIdentityConstraints |
                               XmlSchemaValidationFlags.ProcessInlineSchema |  XmlSchemaValidationFlags.ProcessSchemaLocation;

                StringReader r = new StringReader(xmlData.GetDataString());
                using (XmlReader reader = XmlReader.Create(r, rs))
                {
                    try
                    {
                        while (reader.Read()) { }
                    }
                    catch (XmlException e)
                    {
                        verificationErrorsList.Append(e.Message);
                        verificationErrorsList.Append(Environment.NewLine);
                    }

                }
            }
            catch (XmlSchemaValidationException e)
            {
                verificationErrorsList.Append(e.Message + Environment.NewLine);
            }
        }
        void rs_ValidationEventHandler(object sender, System.Xml.Schema.ValidationEventArgs e)
        {
            if (e.Severity == XmlSeverityType.Error || e.Severity == XmlSeverityType.Warning)
            {
                verificationErrorsList.Append(e.Message + Environment.NewLine);
            }
        }
    }

Usage:

        IData data = new XmlFileData(basepathXML + fileNameXML);
        XMLValidator validator = new XMLValidator();
        validator.ValidationSchema = basepathXML + fileNameXSD;
        validator.Validate(data);
        if (validator.HasErrors)
        {
            Console.WriteLine(validator.ValidationErrors);
        }
        else
        {
            Console.WriteLine("Success");
        }
\$\endgroup\$
10
\$\begingroup\$

A few things:

  1. IData is not a particularly good name for the interface as it represents a source of data rather than the data itself. Therefor IDataSource seems more appropriate.

  2. The interface is a bit ambiguous - which method am I supposed to use GetDataStream or GetDataString? Why use one or the other? Could I call both multiple times and interchangeably?

    As of now it doesn't really provide enough value to warrant it's existence. You'd be better of having XmlValidator accept a TextReader and let the calling code deal with how to wrap it around the source.

  3. Consider passing the ValidationSchema as part of the constructor for the XmlValidator - it's an integral part of its validation logic.

  4. In the validator when the reader throws an exception because there was a problem reading from the stream for example then you catch those as validation errors which I'd consider technically wrong. Your validator should catch validation errors - i.e. anything that has to do with invalid XML. Everything else should not be of its concern. So don't catch Exception - be more specific.

\$\endgroup\$
  • \$\begingroup\$ 2. i wanted to generalize the xml data file, for the sake of completeness, maybe some day the data source might be a url, a network stream, an encripted file? Regarding those to methods.. The GetDataStream() method is has no use right now .. 1,3 done. 4. Ok.. So you say I should only catch validation exceptions in the validator.. so does my updated code meet that point? \$\endgroup\$ – AlexandruC Jan 29 '14 at 15:08
  • \$\begingroup\$ @A.K: Re 2) Sure but your interface should hide that. You don't want a call for every possible type of source on your interface. It should have a generic method which supplies data regardless of the source. TextReader is not an interface but a generic enough abstract base class which should serve the purpose well. \$\endgroup\$ – ChrisWue Jan 30 '14 at 6:59
10
\$\begingroup\$

There is a lot that can be simplified. AppendLine() instead of Append() with Environment.NewLine, object initializer syntax, etc.

ws = new XmlWriterSettings is never used.

Also, StringReader is also an IDisposable resource and should therefore be wrapped in using.

I've also put the very short validation event handler method inline with a lambda.

public interface IData
{
    Stream GetDataStream();

    string GetDataString();
}

internal class XmlValidator
{
    private readonly StringBuilder verificationErrorsList = new StringBuilder();

    public string ValidationSchema
    {
        get;

        set;
    }

    public string ValidationErrors
    {
        get
        {
            return this.verificationErrorsList.ToString();
        }
    }

    public bool HasErrors
    {
        get
        {
            return this.verificationErrorsList.Length > 0;
        }
    }

    public void Validate(IData xmlData)
    {
        try
        {
            ////var ws = new XmlWriterSettings { Indent = true };
            var rs = new XmlReaderSettings
            {
                ValidationType = ValidationType.Schema,
                CloseInput = true,
                ValidationFlags = XmlSchemaValidationFlags.ReportValidationWarnings
                    | XmlSchemaValidationFlags.ProcessIdentityConstraints
                    | XmlSchemaValidationFlags.ProcessInlineSchema
                    | XmlSchemaValidationFlags.ProcessSchemaLocation
            };

            rs.ValidationEventHandler += (sender, e) =>
            {
                if (e.Severity == XmlSeverityType.Error || e.Severity == XmlSeverityType.Warning)
                {
                    this.verificationErrorsList.AppendLine(e.Message);
                }
            };
            rs.Schemas.Add(null, this.ValidationSchema);
            using (var r = new StringReader(xmlData.GetDataString()))
            using (var reader = XmlReader.Create(r, rs))
            {
                try
                {
                    while (reader.Read())
                    {
                    }
                }
                catch (Exception ex)
                {
                    this.verificationErrorsList.AppendLine(ex.Message);
                }
            }
        }
        catch (Exception ex)
        {
            this.verificationErrorsList.AppendLine(ex.Message);
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.