Short XML parser

Question

This is my first code in C# ever. It compiles and works as intended (not complete), but I want to see what I'm doing right and wrong as a first-timer.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Xml;
using System.Xml.Schema;
using System.Xml.Serialization;
using System.Xml.Linq;
using System.IO;

namespace WindowsFormsApplication2
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        /// <summary>
        /// Event driven by browsing a file
        /// </summary>
        /// <param name="sender"></param>
        /// <param name="e"></param>
        private void uploadFile_Click(object sender, EventArgs e)
        {
            OpenFileDialog oOpenFileDialog = new OpenFileDialog();
            oOpenFileDialog.Title = "Open XML File";
            oOpenFileDialog.Filter = "XML Files|*.xml";
            if (!oOpenFileDialog.CheckFileExists) { MessageBox.Show("Invalid Filename!"); }
            if (IsLinux) { MessageBox.Show("Linux! Not supported!"); }
            else { oOpenFileDialog.InitialDirectory = @"C:\...too..long"; }

            if (oOpenFileDialog.ShowDialog() == DialogResult.OK)
            {
                buildTree(oOpenFileDialog.FileName.ToString());

                PBC pbcXmlData = Deserialize(oOpenFileDialog.FileName.ToString());
                Console.WriteLine("----------------------------------");
                Console.Write("Validating.......");
                ValidateXml(oOpenFileDialog.FileName.ToString());
                Console.WriteLine();
                Console.WriteLine("File Path: " + oOpenFileDialog.FileName.ToString());
                PrintXmlData(pbcXmlData);
                Console.WriteLine("-----------------------------------");
            }
        }

        /// <summary>
        /// Builds the treeview of the user's XML file
        /// </summary>
        /// <param name="szPbcFilePath">XML Filename</param>
        public void buildTree(String szPbcFilePath)
        {
            XmlDataDocument xmldoc = new XmlDataDocument();
            XmlNode xmlnode;
            FileStream fs = new FileStream(szPbcFilePath, FileMode.Open, FileAccess.Read);
            xmldoc.Load(fs);
            xmlnode = xmldoc.ChildNodes[1];
            treeView.Nodes.Clear();
            treeView.Nodes.Add(new TreeNode(xmldoc.DocumentElement.Name));
            TreeNode tNode;
            tNode = treeView.Nodes[0];
            AddNode(xmlnode, tNode);
        }

        /// <summary>
        /// Helper for buildTree() to add nodes to the treeview
        /// </summary>
        /// <param name="inXmlNode">input XML Node</param>
        /// <param name="inTreeNode">input Tree</param>
        public void AddNode(XmlNode inXmlNode, TreeNode inTreeNode)
        {
            XmlNode xNode ;
            TreeNode tNode ;
            XmlNodeList nodeList ;
            int i = 0;
            if (inXmlNode.HasChildNodes)
            {
                nodeList = inXmlNode.ChildNodes;
                for (i = 0; i <= nodeList.Count - 1; i++)
                {
                    xNode = inXmlNode.ChildNodes[i];
                    inTreeNode.Nodes.Add(new TreeNode(xNode.Name));
                    tNode = inTreeNode.Nodes[i];
                    AddNode(xNode, tNode);
                }
            }
            else
            {
                inTreeNode.Text = inXmlNode.InnerText.ToString();
            }
        }


        /// <summary>
        /// User's PBC.xml is validated against the PBC.xsd schema.
        /// If the PBC is well-formed and valid, writes to console
        /// "Document is valid." Otherwise, it gives an error message.
        /// 
        /// Rules:
        /// 1.) Exactly one PBCVersion, ProjectName, ProjectVersion element
        /// 2.) PBC/Packages/Package/Depedencies/BuildDependency/RelativeSandboxPath element must be unique
        /// 3.) PBC/Targets/Target/TargetName element must be unique
        /// </summary>
        /// <param name="szPbcFilePath">User's PBC.xml file path</param>
        public void ValidateXml(String szPbcFilePath)
        {
            XmlSchemaSet oSchemas = new XmlSchemaSet();
            oSchemas.Add(null, @"C:\Users\Jonathan\Documents\Visual Studio 2010\Projects\WindowsFormsApplication1\PBC.xsd");

            XDocument oPbcDocument = XDocument.Load(szPbcFilePath);
            string szErrorMessage = "";
            oPbcDocument.Validate(oSchemas, (o, e) =>
            {
                szErrorMessage += e.Message + Environment.NewLine;
            });
            Console.WriteLine(szErrorMessage == "" ? "Document is valid" : "Document invalid: " + szErrorMessage);
        }

        public static void PrintXmlData(PBC pbcXmlData) {} **IRRELEVANT*

        /// <summary>
        /// User's PBC.xml is deserialized into a PBC object 
        /// </summary>
        /// <param name="szPbcFilePath">User's PBC.xml file path</param>
        /// <returns>Deserialized PBC.xml of object PBC</returns>
        public static PBC Deserialize(String szPbcFilePath)
        {
            XmlSerializer oXmlDeserializer = new XmlSerializer(typeof(PBC));
            TextReader oTextReader = new StreamReader(szPbcFilePath);
            object oDeserializerObject = oXmlDeserializer.Deserialize(oTextReader);
            PBC oPbcXmlData = (PBC)oDeserializerObject;
            oTextReader.Close();
            return oPbcXmlData;
        }

        /// <summary>
        /// Detects the execution platform at runtime.
        /// PlatformID 4 = Unix
        /// PlatformID 6 = MacOSX
        /// PlatformID 128 = Unix (.NET 1.0 and 1.1)
        /// </summary>
        public static bool IsLinux
        {
            get
            {
                int iPlatformVersion = (int)Environment.OSVersion.Platform;
                return (iPlatformVersion == 4) || (iPlatformVersion == 6) || (iPlatformVersion == 128);
            }
        }
    }

    /// <summary>
    /// Root Element: <PBC>
    /// Parents: <PBCVersion>, <ProjectName>, <ProjectVersion>,
    ///          <Targets>, <Packages>
    /// Children: <Targets/Target>, <Packages/Package>
    /// </summary>
    [XmlRoot("PBC")]
    public class PBC
    {
        [XmlElement("PBCVersion")]
        public String szPBCVersion;
        [XmlElement("ProjectName")]
        public String szProjectName;
        [XmlElement("ProjectVersion")]
        public String szProjectVersion;
        [XmlArray("Targets")]
        [XmlArrayItem("Target")]
        public List<Target> oTargetList = new List<Target>();
        [XmlArray("Packages")]
        [XmlArrayItem("Package")]
        public List<Package> oPackageList = new List<Package>();
    }

    public class Target : Elements
    {
        [XmlElement("TargetName")]
        public String szTargetName { get; set; }
    }

    public class Package : Elements
    {
        [XmlElement("PackageName")]
        public String szPackageName { get; set; }
    }

    public class Elements
    {
        [XmlElement("CommandLine")]
        public String szCommandLine { get; set; }
        [XmlElement("BuildEnvTypeName")]
        public String szBuildEnvTypeName { get; set; }
        [XmlElement("RelativeOutputPath")]
        public String szRelativeOutputPath { get; set; }
        [XmlElement("RelativeLogPath")]
        public String szRelativeLogPath { get; set; }
        [XmlArray("Dependencies")]
        [XmlArrayItem("BuildDependency")]
        public List<BuildDependency> oBuildDependency = new List<BuildDependency>();
    }

    /// <summary>
    /// Parents: <ProjectName>, <ProjectVersion>, <TargetName>, 
    ///          <ProjectBuild>, <RelativeSandboxPath>
    /// </summary>
    public class BuildDependency
    {
        [XmlElement("ProjectName")]
        public String szBdProjectName { get; set; }
        [XmlElement("ProjectVersion")]
        public String szBdProjectVersion { get; set; }
        [XmlElement("TargetName")]
        public String szBdTargetName { get; set; }
        [XmlElement("ProjectBuild")]
        public String szBdProjectBuild { get; set; }
        [XmlElement("RelativeSandboxPath")]
        public String szBdRelativeSandboxPath { get; set; }
    }


}

Don't cast an enum to int like you're doing in IsLinux, you're defeating the purpose of the PlatformID enumeration! Instead of having easily readable, self-documenting, strongly-typed identifiers in your code, you've put magic numbers! — Mathieu Guindon
– Mathieu Guindon, Commented Jul 7, 2014 at 5:25
Here's a great guide to widely accepted C# naming conventions msdn.microsoft.com/en-us/library/ms229043(v=vs.110).aspx — craftworkgames
– craftworkgames, Commented Jul 7, 2014 at 11:13

AlanT · Accepted Answer · 2014-07-07 07:56:28Z

Overall Impression
Applications should be split into layers.
I know it's a simple, throwaway, demo application but these are a perfect opportunity to start working on splitting an application into a Model, View and X. (A Matter Of Personal Preference (AMOPP); looking at the namespaces, you are working with WinForms - I would go with WPF instead. Have a google and see what you think of the arguments)

Naming
Hungarian Notation is not recommended for C# code.
Pascal Naming is recommended for methods (e.g. BuildTree() rather than buildTree() )(though this may just be a typo as the others are Pascal Cased)
PBC is not a useful class name. Something descriptive of the function is better. Similarly Elements, is too vague and general.

Other Things
I find

if (!oOpenFileDialog.CheckFileExists) { MessageBox.Show("Invalid Filename!"); }

puzzling. It has been a while but, as far a I remember, CheckFileExists is a property that one sets before showing the dialog, telling it to display a warning (or not) if the file selected doesn't exist. The code above is not the correct usage (and will never show the "Invalid FileName!" message because the default is true.)

The processing order seems off, you are deserializing the contents of the file (I'd pass in a stream here, rather than a file name. It gives much more flexibility), then validating it - which, AFAIK, should never run if the Xml is invalid because the deserialization will throw an exception (Haven't tested this so I may be wrong, but either way, validating after a successful load seems off).

Grab the file name out of the open dialog and store it so that you do not have to keep using oOpenFileDialog.FileName.ToString()

RubberDuck · Accepted Answer · 2014-07-08 16:19:09Z

4

You have an awful lot of code behind your form. I bet this won't be the only place you'll want to prompt a user for a file and display a tree. Put those methods in their own classes for reusability.
Hungarian notation is not recommended in c#. If you're going to use it, which I recommend against, variable names should indicate their kind instead of their type. Really though, just don't use Hungarian notation. Everyone messes it up.
Take advantage of the fact that c# is case sensitive. FileStream is different from fileStream, so don't use fs. It's over abbreviated.
There's a file path hardcoded into ValidateXml. Use a constant instead. It will be easier to swap out later. The way it is, that function will work only on your machine.

edited Jul 8, 2014 at 16:19

answered Jul 7, 2014 at 11:35

RubberDuck

31.2k6 gold badges74 silver badges177 bronze badges

1

\$\begingroup\$ Do you mean “FileStream is different from fileStream”? \$\endgroup\$

svick
– svick

2014-07-08 16:17:55 +00:00
Commented Jul 8, 2014 at 16:17
\$\begingroup\$ Yes. Thank you for calling that out @svick. \$\endgroup\$

RubberDuck
– RubberDuck

2014-07-08 16:18:48 +00:00
Commented Jul 8, 2014 at 16:18

Add a comment |

Stack Exchange Network

Short XML parser

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Short XML parser

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions