Simple specific classes design

Question

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");
        }

Community · Accepted Answer · 2020-06-10 13:24:26Z

10

A few things:

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.
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.
Consider passing the ValidationSchema as part of the constructor for the XmlValidator - it's an integral part of its validation logic.
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.

edited Jun 10, 2020 at 13:24

CommunityBot

1

answered Jan 28, 2014 at 19:12

ChrisWue

20.6k4 gold badges43 silver badges107 bronze badges

\$\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
– AlexandruC

2014-01-29 15:08:00 +00:00
Commented Jan 29, 2014 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
– ChrisWue

2014-01-30 06:59:32 +00:00
Commented Jan 30, 2014 at 6:59

Add a comment |

Jesse C. Slicer · Accepted Answer · 2014-01-28 19:44:52Z

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);
        }
    }
}

Stack Exchange Network

Simple specific classes design

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Simple specific classes design

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions