Displaying data from a database onto a form

Question

I'm a beginner to .NET and could you guide me to right direction. My problem is based on the following code. Here I have 4 variations of same method and all 4 variations are working fine.

1. What is the recommended or standard way of doing this?

2. If none of these methods are okay, kindly suggest one with code?

Code explanation:

From a windows form I'm calling a viewAccount() method which is in bankAccount class. Its purpose is to get relevant bank account details of an employee from the database and then those details should be shown in the text boxes of calling form.

Also please note that I have reduced no of line to make it more readable.

Example 01 - Method will return a bankAccount object with fields populated with data from the database

class bankAccount
{
    //Member fields...
    string acNo;
    string acName;
    string bank;
    string acType;
    frmShowAccount form=new frmShowAccount();

    public bankAccount viewAccount( string acNo )
    {
        this.acNo = acNo;

        using (SqlConnection newCon = new SqlConnection(db.GetConnectionString))
        {
            SqlCommand newCmd = new SqlCommand("SELECT Employee.Name, BankAccount.ac_name, BankAccount.bank_name, BankAccount.ac_type FROM BankAccount INNER JOIN Employee ON BankAccount.emp_id = Employee.Emp_ID WHERE (BankAccount.ac_no = @bankAccount)", newCon);

            newCmd.Parameters.Add("@bankAccount", SqlDbType.Char).Value = acNo;
            newCon.Open();
            SqlDataReader rdr = newCmd.ExecuteReader();
            rdr.Read();

            form.txtEmpName.text = rdr.GetString(0); //EmpName is not a member of bankAccount class
            this.acName = rdr.GetString(1);
            this.bank = rdr.GetString(2);
            this.acType = rdr.GetString(3);

            return this;
        }
    }
}

// CALLING THE ABOVE METHOD...

bankAccount newBA = new bankAccount();
newBA = newBA.viewAccount(txtACNo.text);  // A reference is set to the instance returned
txtACName.text = newBA.acName;  // Get the value of instance field to text box

Example 02 - Method will return a data reader and it will be used by the form to get data

    class bankAccount
    {
        string acNo;
        string acName;
        string bank;
        string acType;

        public SqlDataReader viewAccount( string acNo )
        {
            this.acNo = acNo;

            using (SqlConnection newCon = new SqlConnection(db.GetConnectionString))
            {
                SqlCommand newCmd = new SqlCommand("Same SELECT …”,newCon);

                newCmd.Parameters.Add()…
                newCon.Open();
                SqlDataReader rdr = newCmd.ExecuteReader();
                rdr.Read();

                return rdr;
            }
        }
    }

//CALLING THE ABOVE METHOD...
bankAccount newBA = new bankAccount();
SqlDataReader rdr = newBA.viewAccount(txtACNo.text) //A reference to hold the returning reader from the method call
txtACName.text = rdr.getString(1); //Get the value through the reader to text box

Example 03: this method want return values but explicitly assign values to the text boxes in the form

  class bankAccount
  {
        string acNo;
        string acName;
        string bank;
        string acType;
        frmShowAccount form=new frmShowAccount();

        public void viewAccount( string acNo )
        {
            this.acNo = acNo;

            using (SqlConnection newCon = new SqlConnection(db.GetConnectionString))
            {
                SqlCommand newCmd = new SqlCommand("Same SELECT …", newCon);

                newCmd.Parameters.Add()…
                newCon.Open();
                SqlDataReader rdr = newCmd.ExecuteReader();
                rdr.Read();

                // Setting values to the text boxes in the current instance of form
                form.txtName.text=rdr[0];
                form.txtACName.text=rdr[1];
                form.txtBankName.text=rdr[2];
                form.txtACType.text=rdr[3];         
            }
        }
    }

//CALLING THE ABOVE METHOD
bankAccount newBA = new bankAccount();
newBA.form.this; // reference 'form' which is in the 'bankAccount' class is set to current instance of the form object.

Example 04: this method want return any value. It will only initialize instance fields of the class with the data

    class bankAccount
    {
        string acNo;
        string acName;
        string bank;
        string acType;
        frmShowAccount form=new frmShowAccount();

        public void viewAccount( string acNo )
        {
            this.acNo = acNo;

            using (SqlConnection newCon = new SqlConnection(db.GetConnectionString))
            {
                SqlCommand newCmd = new SqlCommand("Same SELECT …)", newCon);

                newCmd.Parameters.Add()…
                newCon.Open();
                SqlDataReader rdr = newCmd.ExecuteReader();
                rdr.Read();

                form.txtName.text=rdr[0];
                this.acName=rdr[1];
                this.bank=rdr[2];
                this.acType=rdr[3];
        }
    }


// CALLING THE ABOVE METHOD
bankAccount newBA = new bankAccount();
txtACName.text = newBA.acName; // Text boxes get the data from account object's instance fields (probably through a get property)

Answer 1

None of the Above.

You've shown 4 different ways of mixing up presentation and data concerns; the 4 approaches only differ in minute implementation details and all suffer the same flaws - what you have here is a God class that knows everything about everything there is to know.

Let's think differently.

---

This answer uses the names I would have used instead of yours.

---

We have at least 3 distinct, separate things:

• We have a Form that knows about its textboxes.

• We have a BankAccountInfo that holds an AccountNumber, an AccountName, an AccountType the BankName and an EmployeeName.

• We have a way to fetch data from the database, here ADO.NET.

Each of these 3 concepts is its own class - you have the Form and the BankAccount, but you've turned the bank account into some "coordinator" that knows about the Form and deals with fetching the data from the database.

Leaving the form aside, BankAccountInfo should look like this:

public class BankAccountInfo
{
    public string AccountNumber { get; set; }
    public string AccountName { get; set; }
    public string AccountType { get; set; }
    public string BankName { get; set; }
    public string EmployeeName { get; set; }
}

Where's the code? Elsewhere. It's not the role of this object to know about textboxes, nor about some SQL backend. It has only one, simple goal: expose data. Not fetch data - that's the role of a service class:

public class BankAccountDataService
{
    private readonly string _connectionString;
    public BankAccountDataService(string connectionString)
    {
        _connectionString = connectionString;
    }

    public BankAccountInfo GetByAccountNumber(string accountNumber)
    {
        using (var connection = new SqlConnection(_connectionString))
        {
            var sql = "SELECT ...";
            connection.Open();
            using (var command = new SqlCommand(sql, connection))
            {
                command.Parameters.AddWithValue(...);
                using (var reader = command.ExecuteReader())
                {
                    if (reader.Read())
                    {
                        return new BankAccountInfo
                                   {
                                       EmployeeName = reader[0].ToString(),
                                       AccountNumber = reader[1].ToString(),
                                       AccountName = reader[2].ToString(),
                                       BankName = reader[3].ToString(),
                                       AccountType = reader[4].ToString()
                                   }; 
                    }
                }
            }
        }

        return null;
    }
}

Now you have a class whose role is specifically to interact with the database; its interface gives you a GetByAccountNumber method that lets you fetch everything there is to know about a BankAccountInfo by passing in an account number. It knows nothing of a form, and it doesn't care how the returned object is being used, it's none of its concern.

This is where you have a design decision to make: you need to somehow "connect" these pieces together. Let's see:

• the form really only needs to know about BankAccountInfo.
• the BankAccountInfo doesn't need to know about BankAccountDataService.
• the BankAccountDataService needs to know about BankAccountInfo (and any other type it might return).

We're missing something else.

public class BankAccountPresenter
{
    private BankAccountView _form;
    private BankAccountDataService _service;

    public BankAccountPresenter(BankAccountView form, BankAccountDataService service)
    {
        _form = form;
        _form.OnShowAccountInfo += View_OnShowAccountInfo;

        _service = service;
    }

    public void Show()
    {
        _form.ShowDialog();
    }

    private View_OnShowAccountInfo(object sender, EventArgs e)
    {
        var info = _service.GetByAccountNumber(_form.AccountNumber);
        if (info == null)
        {
            MessageBox.Show("Could not find an account for specified number.");
            return;
        }

        _form.AccountName = info.AccountName;
        _form.AccountType = info.AccountType;
        _form.BankName = info.BankName;
    }
}

The above assumes that there's another class that's calling the presenter's Show method (that class will own the form and the service instances), and that the BankAccountView class looks something like this:

public partial class BankAccountView : Form
{
    public event EventHandler OnShowAccountInfo;

    public string AccountNumber
    {
        get { return AccountNumberTextBox.Text; }
        set { AccountNumberTextBox.Text = value; }
    }

    public string AccountName
    {
        get { return AccountNameTextBox.Text; }
        set { AccountNameTextBox.Text = value; }
    }

    public string AccountType
    {
        get { return AccountTypeTextBox.Text; }
        set { AccountTypeTextBox.Text = value; }
    }

    public string BankName
    { 
        get { return BankNameTextBox.Text; } 
        set { BankNameTextBox.Text = value; }
    }

    public BankAccountView()
    {
        InitializeComponents();
    }

    private void GetAccountInfoButton_Click(object sender, EventArgs e)
    {
        if (OnShowAccountInfo != null)
        {
            OnShowAccountInfo(this, EventArgs.Empty);
        }
    }
}

---

What's going on here? The view does nothing other than displaying the data and notifying the presenter whenever it needs something to happen - here, the user clicked a button: the presenter does all the work, fetching the required model from the database using an instance of a service class. Everyone plays its own little part, and everything is properly encapsulated - you don't expose the form's TextBox members, rather, you expose a string that represents that textbox' value.

---

Your Code

I already commented on that, but I'll reiterate it: I like that you're using parameters. Lots of beginners will just concatenate them into the command string, and that tends to make my eyes bleed that's rather ugly.

Here's a brain-dump of what jumped at me while I was reviewing your code:

• Java-style naming. camelCase in C# is for locals (parameters, etc.) - class, method and property names use PascalCase.
• Hungarian Notation and Disemvoweling. Don't chop off vowels in your identifiers. Use names that you can pronounce. Since this is WinForms there's some kind of a historical consensus about the accepted semi-Hungarian naming style for controls though.
• Undisposed disposables. You're using using blocks and that's great, however you're not consistently disposing all objects that implement IDisposable, that's less great.
• Misleading names. The ViewAccount(string) method doesn't actually display anything, it fetches information. Do what you say, say what you do.

Answer 2

with all of the methods you should add another using statement for your Readers as well.

right now you aren't closing your reader, I am not sure that the reader is closed inside the using block of the connection, but I would imagine it should be, logically you can't have the reader after the connection is closed so I would think it would close the reader automatically. The only thing with that is you don't see it in the code so you can't be sure that is what it is doing.

If I were you I would just wrap it in a using statement.

---

you should also use a using statement with

SqlCommand newCmd = new SqlCommand("Same SELECT …)", newCon);

like this

using (SqlConnection newCon = new SqlConnection(db.GetConnectionString)) {
    using (SqlCommand newCmd = new SqlCommand("Same SELECT …)", newCon) {
        newCmd.Parameters.Add()…
        newCon.Open();
        Using (SqlDataReader rdr = newCmd.ExecuteReader()) {
            rdr.Read();

            form.txtName.text=rdr[0];
            this.acName=rdr[1];
            this.bank=rdr[2];
            this.acType=rdr[3];
        }
    }
}