views:

145

answers:

1

I've got a DataGridView using a DataTable as it's DataSource. I'm allowing the user to alter the visibility, order (and width though that's not relevant to this question) of the DataGridView columns, but not the underlying DataTable. This means I don't have to recreate the DataTable every time the user makes a change to the view.

The user can export the DataTable to an XML file and I'd like the output to reflect what's on the screen. I originally had the following:

dataTable.WriteXml(saveFileDialog.FileName, XmlWriteMode.WriteSchema);

But this (obviously) writes all of the columns in the DataTable to file including the invisible ones in the order they're defined.

I've got the following code in the export button handler method. I know that it's in the wrong place, but I'm stumped as it needs to reference the DataGridView.

private void export_Click(object sender, EventArgs e)
{
    // Show the SaveFileDialog.
    var saveFileDialog = new SaveFileDialog();
    saveFileDialog.Title = Properties.Resources.TSaveLibrary;
    saveFileDialog.Filter = Properties.Resources.TXMLFilter;
    if (saveFileDialog.ShowDialog() == DialogResult.OK)
    {
        using (XmlWriter writer = XmlWriter.Create(saveFileDialog.FileName))
        {
            writer.WriteStartDocument();
            writer.WriteRaw("\n");
            writer.WriteStartElement("Library");
            writer.WriteRaw("\n");

            // Find the visible columns and put them in display order
            var dcCollection = new List<string>();
            DataGridViewColumn dc = dataGridView.Columns.GetFirstColumn(DataGridViewElementStates.Visible);
            while (dc != null)
            {
                if (dc.Visible)
                {
                    dcCollection.Add(dc.Name);
                }
                dc = dataGridView.Columns.GetNextColumn(dc, DataGridViewElementStates.Visible, DataGridViewElementStates.None);
            }

            // Write the data
            foreach (DataRow dr in dataTable.Rows)
            {
                writer.WriteRaw("  ");
                writer.WriteStartElement(dataTable.TableName);
                writer.WriteRaw("\n");

                foreach (string colName in dcCollection)
                {
                    writer.WriteRaw("    ");
                    writer.WriteElementString(colName, dr[colName].ToString());
                    writer.WriteRaw("\n");
                }

                writer.WriteRaw("  ");
                writer.WriteEndElement();
                writer.WriteRaw("\n");
            }

            writer.WriteEndElement();
            writer.WriteRaw("\n");
            writer.WriteEndDocument();
        }
    }
}

Any suggestions on how I can refactor this code?

+1  A: 

If this code works, (and it seems like it should) then you're good. The only issue I sense you're seeing is that you feel this code shouldn't be in the UI layer. Well you don't have many options as you kinda need the DGV in order to properly build up the XML file based on what's currently visible in the grid. Therefore, if you're using .NET 3.5 I'd say put this in a seperate class as an extension method to the datagridview something like:

public static class DataGridViewExtensions
{
   public static void SaveAsXml(this DataGridView grid, string path)
   {
      //do work
   }
}

Or, if you're not using 3.5, then just have a static class anyway with a similiar approach, but actually pass in the DGV something like:

public static class DataGridViewHelper
{
   public static void SaveAsXml(DataGridView grid, string path)
   {
      //do work
   }
}

A third option, and I personally wouldn't do it this way, is to create a NEW datatable based on what's currently visible on the grid, and call WriteXml method on that table.

BFree
@BFree - yes the code works but I feel the writing code should be in a separate class - separation of concerns and all that.
ChrisF
Finally come back to this. BFree is right, there's no right answer, but the point about having to have access to the DGV means that what I've got is pretty much the least worst solution. So I'm going to accept this answer.
ChrisF