HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMajor

Displaying data from a database onto a form

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
displayingdatabaseontoformfromdata

Problem

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.

-
What is the recommended or standard way of doing this?

-
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
txtA

Solution

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

Code Snippets

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

Context

StackExchange Code Review Q#46985, answer score: 20

Revisions (0)

No revisions yet.