0
\$\begingroup\$

I have a very basic method at the moment that fills text boxes with data relevant to the selected company. I load the data into a DataGrid using this method;

CompanyDataService

namespace SdcDatabase.Controller
{
    class CompanyDataService
    {
        public DataTable GetCompanyList()
        {
            string constr = ConfigurationManager.ConnectionStrings["dbfString"].ConnectionString;
            using (var dbfCon = new OleDbConnection(constr))
            {
                dbfCon.Open();
                using (var myQuery = dbfCon.CreateCommand())
                {
                    myQuery.CommandText = "SELECT cm_name, cm_addr1, cm_addr2, cm_town, cm_county, cm_pcode, cm_tel1, cm_tel2, cm_fax, cm_email FROM compns ORDER BY cm_name";
                    var dTable = new DataTable();
                    var dataAdapter = new OleDbDataAdapter(myQuery);
                    dataAdapter.Fill(dTable);
                    return dTable;
                }
            }
        }
    }
}

After the data is inside the DataGrid I then load the data into the textboxes based upon the company selected. I do this using this method;

FillCompanyListTextBoxes

        private void FillCompanyListTextBoxes(object sender, SelectionChangedEventArgs e)
    {
        DataRowView dv = (DataRowView)dataGrid.SelectedItem;
        if (dv != null)
        {
            FillCompanyDetailsTextBoxes();
            compDetailsLabel.Content = "Details for: " + dv.Row.ItemArray[0].ToString();
            compNameTextBox.Text = dv.Row.ItemArray[0].ToString(); //Company Name
            compAddr1TextBox.Text = dv.Row.ItemArray[1].ToString(); //Company Addr1
            compAddr2TextBox.Text = dv.Row.ItemArray[2].ToString(); //Company Addr2
            compTownTextBox.Text = dv.Row.ItemArray[3].ToString(); //Company Town
            compCountyTextBox.Text = dv.Row.ItemArray[4].ToString(); //Company County
            compPcodeTextBox.Text = dv.Row.ItemArray[5].ToString(); //Company Post Code
            compTelTextBox.Text = dv.Row.ItemArray[6].ToString(); //Company Telephone
            compAltTelTextBox.Text = dv.Row.ItemArray[7].ToString(); //Company Alt Tel
            compFaxTextBox.Text = dv.Row.ItemArray[8].ToString(); //Company Fax
            compEmailTextBox.Text = dv.Row.ItemArray[9].ToString(); //Company Email
        }
    }

FillCompanyDetailsTextBoxes is simply a method that loads that data into textboxes in the next tabitem, allowing the user update the companies details if necessary.

I know the second method isn't great. It works and it doesn't look overly disturbing but many problems would be encountered if for some reason the column numbers changed and then all the text boxes would have a domino effect and all be in the wrong place.

Basically my question is this, is there a better way?

\$\endgroup\$
2
  • \$\begingroup\$ Code formatting: codereview.stackexchange.com/editing-help Easiest is simply to select the code and then click the button with the {}. \$\endgroup\$
    – BCdotWEB
    Commented Oct 16, 2015 at 11:26
  • \$\begingroup\$ @BCdotWEB as weird as this may sound I don't have that button when I post questions. Is there a reason for this? \$\endgroup\$
    – CBR
    Commented Oct 16, 2015 at 12:16

1 Answer 1

4
\$\begingroup\$

Like I've said in an answer to a previous question of yours: implement MVVM. Do not work in the code-behind, instead bind from the XAML in your View to its ViewModel.

But more importantly: first you need to stop using DataTables and start using well-defined objects. Your code already shows that currently you need to add comments to remember that for instance the sixth item of an ItemArray that's attached to your row contains the Company Telephone. Now what if you simply had a List<Company> where each Company had a property called Telephone -- wouldn't that be far easier and less error-prone?

Note that your code still contains the same bad practices we warned about in your two previous questions, e.g. OleDbDataAdapter is IDisposable so it needs to be inside a using. You need to apply our remarks not just to the small bit of code you offer for review, but you need to apply those lessons to all of your code.

\$\endgroup\$
4
  • \$\begingroup\$ I appreciate you taking time to answer and offer me advice. One thing I am unsure of is if I had a Company model for example that had getters/setters for all their details, how would I get the details populated by the OleDbQuery into the DataGrid? Instead of placing the results into a DataTable do I need to create a new instance of the model class and populate a list created with that? \$\endgroup\$
    – CBR
    Commented Oct 16, 2015 at 12:31
  • \$\begingroup\$ @CBR Yes, that's the way to go. Even better would be to use an ORM, perhaps something like github.com/StackExchange/dapper-dot-net . \$\endgroup\$
    – BCdotWEB
    Commented Oct 16, 2015 at 13:10
  • \$\begingroup\$ Ok thanks for all your answers. One issue I would have on that solution is how do I place the contents of a selected row into the TextBoxes still? Do I still need to create a DataRowView on the selecteditem? EDIT: please ignore this comment, I have found the easy way compNameTextBox.Text = compDetails.CompanyName; I'll leave it here just incase it has already been viewed \$\endgroup\$
    – CBR
    Commented Oct 16, 2015 at 13:12
  • \$\begingroup\$ @CBR IIRC DataGrid has such edit functions built-in, but it's not something I've done in a while. Or you could work with DataGridTemplateColumn (CellTemplate vs CellEditingTemplate) etc. \$\endgroup\$
    – BCdotWEB
    Commented Oct 16, 2015 at 13:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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