Accessibility of class in multiple forms

[Teranoz]

class cConnection
{
string MiningConString = @"data source=" + AppDomain.CurrentDomain.BaseDirectory + "miningdata.sdf";
public SqlCeConnection CN;
public SqlCeCommand Cmd;
public void OpenCon()
{
CN = new SqlCeConnection(MiningConString);
CN.Open();
Cmd = new SqlCeCommand();
Cmd.Connection = CN;
}

public void CloseCon()
{
Cmd.Dispose();
CN.Close();
CN.Dispose();
}
}

public class cSetting
{
public int ChatboxBottom = 0;
public int DeedBottom = 0;
public int MapSize = 1000;

public void GetSetting()
{
DataSet ds = new DataSet();
SqlCeDataAdapter adapter = new SqlCeDataAdapter();
DataTable dt;
cConnection CN = new cConnection();
CN.OpenCon();
String SQL = "Select * from tblSettings";
CN.Cmd.CommandText = SQL;
adapter.SelectCommand = CN.Cmd;
adapter.Fill(ds);
dt = ds.Tables[0];
String SettingName;
foreach (DataRow da in dt.Rows)
{
SettingName = da["settingname"].ToString();
switch (SettingName)
{
case "Chatbox bottom":ChatboxBottom = (int)da["intvalue"]; break;
case "Deed bottom": DeedBottom = (int)da["intvalue"]; break;
case "Map size (meters)": MapSize = (int)da["intvalue"]; break;
}

}
CN.CloseCon();
}

}

public partial class frmMain : Form
{
public cSetting Setting = new cSetting();
public frmMain()
{
InitializeComponent();
}

private void frmMain_Load(object sender, EventArgs e)
{
Setting.GetSetting();
}

private void settingsToolStripMenuItem_Click(object sender, EventArgs e)
{
frmSettings frmS = new frmSettings();
frmS.Show();
}
}
public partial class frmSettings : Form
{
public frmSettings()
{
InitializeComponent();
}

private void frmSettings_Load(object sender, EventArgs e)
{
//do something with frmMain.Setting
}
}


How can I access frmMain.Setting in frmSettings (and is the class structure of my program right or could it be coded better)

[BigEd781]

This is a very common question from people who are just learning the language, and there are a couple of ways to solve it. It is hard to give you specific information because you do not say why you need to access that public variable, but some general solutions may be...

1. Use the constructor instead of the Load event.

See if you can just specify a parameter in fmrSetting's constructor. Do you need access to the entire "cSetting" object, or do you just need some information from it? For example, perhaps you can just pass in a few properties of the cSetting object to get all of the data that you need. Perhaps you just pass in the cSetting itself, but I wouldn't recommend passing around public instance members if you can avoid it.


public frmSettings( int someValue, string someOtherValue, ... ) { ... }

2. Use events.

If you need to keep around a reference to the main form, you could also pass that in as a constructor parameter. However, this is generally not considered good practice and can lead to some hard to maintain code later in the project's life. However, you can usually accomplish the same thing by using events.

Say you open a sub form so that a user can enter some information. For the sake of this example, let's say you are opening a login form. Now, you are tempted to do something like this:


class LoginForm : Form
{
private MainForm _mainForm;

public LoginForm( MainForm mainform )
{
_mainForm = mainform;
}

private void Button_Click( object sender, EventArgs e )
{
// get username and password in some variables here

_mainForm.Username = username;
_mainForm.Password = password;
}
}

class MainForm : Form
{
private void frm_AttemptLogin( object sender, LoginEventArgs e )
{
using ( LoginForm frm = new LoginForm( this ) )
{
frm.ShowDialog( );

// when we return to here our username and password have already been set.
// so, if we wanted to validate them, we would have to open a new LoginForm
// dialog after the validation routine fails.
}
}
}

So, we took in a reference to a MainForm in the LoginForm constructor and then set the properties of the MainForm when the user clicked the accept button. It works, and for now it is not a problem. However, in the future the Loginform class starts growing and gaining more control over your MainForm instance. This increases coupling, which increases headaches, which increases schedules, etc.

Now, if had used an event driven approach, we could have done something like this:


class LoginEventArgs : EventArgs
{
public readonly string Username;
public readonly string Password;

public bool Accepted { get; set; }

public LoginEventArgs( string username, string password )
{
Username = username;
Password = password;
}
}

class LoginForm : Form
{
public event EventHandler<LoginEventArgs> AttemptLogin;

protected virtual void OnAttemptLogin( LoginEventArgs e )
{
if ( AttemptLogin != null )
{
AttemptLogin( this, e );
}
}

private void Button_Click( object sender, EventArgs e )
{
// get username and password in some variables here

OnAttemptLogin( new LoginEventArgs( username, password ) );
}
}

class MainForm : Form
{
private void buttonLogin_Click( object sender, EventArgs e )
{
using ( LoginForm frm = new LoginForm( ) )
{
frm.AttemptLogin = frm_AttemptLogin;
frm.ShowDialog( );
}
}

private void frm_AttemptLogin( object sender, LoginEventArgs e )
{
// We now have a chance to validate the credentials before
// any data can be set. We can set a property and return early
// if the credentials are invalid.
if ( !IsValid( e.Username, e.Password )
{
// the LoginForm can now show an error message without closing
// first by checking this property after the event has been fired.
e.Accepted = false;
return;
}

// Now we can store the username and password as private
// variables. No chance they can be changed from the outside.
_username = e.Username;
_password = e.Password;
}
}


This approach requires a bit more code, but it is cleaner (there is less coupling between classes), more powerful, and easier to maintain.

Since you asked about possible improvements to your code, I can point out a couple of things:

1. Don't use public instance variables.

With the support for properties in the language there is really no reason to have public variables. Just wrap those in a property and adding logic to the getter and setter becomes trivial.

2. Call Dispose() on objects that implement IDisposable.

If a class implements the IDisposable interface they do so because they are holding references to resources that will not be cleaned up automatically by the garbage collector. There are other reason to implement the interface of course, but the point is that these classes are telling its clients that they need to call the Dispose() method before the object goes out of scope. when you do this:


frmSettings frmS = new frmSettings();
frmS.Show();


You create a new form that goes out of scope by the end of the method, but you never call the Dispose() method. This is a possible memory leak. Because the form is called with the Show() method it is not modal, so you would need to keep an instance reference to it to make sure that you can call Dispose() some time in the future. If you were displaying a modal form you would just write this:


using ( frmSettings frmS = new fmrSettings( ) )
{
fmrS.ShowDialog( );
}
// frmS is disposed.


If you haven't seen a 'using' statement before, it is just syntactic sugar for this:


try
{
frmSettings frmS = new fmrSettings( );
fmrS.ShowDialog( );
}
finally
{
fmrS.Dispose( );
}