c# - Accessing class from form and vice-versa

Question

After hunting the internet for two days and not finding a solution that I can understand properly I have to ask on here for an answer.

I have a windows forms application that was written in vb.net and works fine. I have decided to rewrite this in c# which I thought wouldn't be too much of a problem but ...

I have two classes in the project :

FormJobs & AppJobs

FormJobs contains methods and functions that modify the forms in some way.

AppJobs contains methods and functions for everything else (Checks,Scanning and so forth).

On my main form (FrmStart) the On_load event uses a function from AppJobs to check that the network is up (public bool CheckNetConnection) and then Checks to make sure that the root save folder exists (public void CheckRoot).

If CheckNetConnection is false or CheckRoot does not exist then a method in the FormJobs class sets some buttons to disabled, some labels to display information as to what has gone wrong and also sets the height of the form.

The above works in VB.net but I keep getting a StackOverflowException or NullReferenceException with the C# code.

I know the reason for the Exceptions is because the two classes and the form all keep calling each other so I know that I need to remove this code but I am not sure how to let each class and the form access each other. It is obviously bad design as I`m just starting to learn C# so any help on this would be much appreciated.

But my main questions are:-How do I get a form to access multiple classes? Allow the classes to access each other? Let the classes make changes to a form?

FrmStart Code

AppJobs Appjobs = new AppJobs();

private void FrmStart_Load(object sender, EventArgs e)
    {

                    KeyPreview = true;

        if (Appjobs.CheckNetConnection(this) == true)
        {
            Appjobs.CheckRoot(this);
        }

AppJobs Code

public class AppJobs
{

    FormJobs Formjobs = new FormJobs();

    public string AppRoot = Properties.Settings.Default.DefaultFolder;
    public string DefaultDevice = Properties.Settings.Default.DefaultScanner;
    public bool NoDirectory = false;

    DialogResult MsgBoxQuestion;

    public bool CheckNetConnection(Form StartForm)
    {

        IPHostEntry ServerIP = new IPHostEntry();
        bool ConnectedToServer = false;
        string CurrentRoot = "MyServer";

        if (System.Net.NetworkInformation.NetworkInterface.GetIsNetworkAvailable())
        {
            try
            {
                IPHostEntry DNSTest = Dns.GetHostEntry(CurrentRoot);
                if (DNSTest.AddressList.Length > 0)
                {
                    ConnectedToServer = true;
                }
                else
                {
                    ConnectedToServer = false;

                }


            }
            catch (System.Net.Sockets.SocketException ex)
            {
                ConnectedToServer = false;
            }
        }

        return ConnectedToServer;

    }

    public void CheckRoot(Form StartForm)
    {
        if (string.IsNullOrEmpty(AppRoot))
        {
            Formjobs.SetHeight(StartForm);
            return;


        }else if(AppRoot == "0")
        {
            Formjobs.SetHeight(StartForm);
            return;
        }
        else
        {
            if ((!Directory.Exists(AppRoot)))
            {
                NoDirectory = true;
                MsgBoxQuestion = MessageBox.Show(AppRoot + " is set, but the directory does not exist." + Environment.NewLine
                    + Environment.NewLine + "Would you like to create the folder now?", "Root folder missing", MessageBoxButtons.YesNo);
                if (MsgBoxQuestion == DialogResult.Yes)
                {
                    Directory.CreateDirectory(AppRoot);
                    NoDirectory = false;
                }
                else
                {
                    MessageBox.Show("You will not be able to use this program until you create a root folder.", "No root folder selected",MessageBoxButtons.OK);
                }

            }

        }

    }
}

FormJobs Code

public class FormJobs
{

    AppJobs Appjobs = new AppJobs();

    public void SetHeight(Form StartForm)
    {

        if (Appjobs.AppRoot == null | Appjobs.AppRoot == "0") {

if (Appjobs.DefaultDevice == null | Appjobs.DefaultDevice == "0") {

    if (StartForm.Controls["MenuStrip1"].Visible == true) {
        StartForm.Height = 167;
        StartForm.Controls["LblNoRoot"].Visible = true;
        StartForm.Controls["LblNoRoot"].Location = new Point(0, 24);
        StartForm.Controls["LblNoRoot"].Text = "There is no root folder selected. Please select a root folder to continue.";
        StartForm.Controls["LblNoDevice"].Visible = true;
        StartForm.Controls["LblNoDevice"].Location = new Point(0, 48);
        StartForm.Controls["LblNoDevice"].Text = "There is no default device selected. Please select a default device to continue.";
        StartForm.Controls["BtnOkTickets"].Enabled = false;
        StartForm.Controls["BtnQueryTickets"].Enabled = false;
        StartForm.Controls["BtnSearch"].Enabled = false;

    }else

        {
        StartForm.Height = 147;
        StartForm.Controls["LblNoRoot"].Visible = true;
        StartForm.Controls["LblNoRoot"].Location = new Point(0, 9);
        StartForm.Controls["LblNoRoot"].Text = "There is no root folder selected. Please select a root folder to continue.";
        StartForm.Controls["LblNoDevice"].Visible = true;
        StartForm.Controls["LblNoDevice"].Location = new Point(0, 33);
        StartForm.Controls["LblNoDevice"].Text = "There is no default device selected. Please select a default device to continue.";
        StartForm.Controls["BtnOkTickets"].Enabled = false;
        StartForm.Controls["BtnQueryTickets"].Enabled = false;
        StartForm.Controls["BtnSearch"].Enabled = false;

        }


}

Answer 1

One of the causes of your problems is that everyone is changing your StartForm. Apart from that this spaghetti makes it difficult to understand, it certainly doesn't help to make your classes reusable and maintainable if your Startform changes.

It seems to me, that AppJobs is meant to decide what the form should look like (for instance it decides that the StartForm should change height), while FormJobs performs the calculations needed to change this height. StartForm apparently is just allowing to let everyone make changes to him.

A better design would be that StartForm would not ask AppJobs to change its size, and to ask the operator whether a folder should be generated. Instead if ought to ask appJobs for advise: "Which height do you think I should have". after that it could ask FormJobs: "Please adjust my height according to this specification"

FormJobs should trust StartForm that it has gathered the correct information about how a StartForm ought to look like. FormJobs should not ask AppJobs for any information: "Hey AppJobs, StartForm asked me to change its appearance to certain specifications, but I'm not certain whether StartForm has done its job correctly. Please tell me if these specifications are correct, and give me some missing information")

The correct division into tasks would be:

• AppJobs specifies the format of any StartForm according to its internal state (a.o. AppRoot, and existence of certain folders)
• StartForm is the one who displays all items. He decides who to ask for specifications, and what to do with the returned specifications. He is also the only one who communicates with operators
• FormJobs is a class that apparently knows all elements from a StartForm. If you will only have one type of StartForm, then Appjobs should be part of the Startform class. If you think there might be several different Startform classes, all with the same elements that ought to be manipulated similarly, shouldn't all these StartForm classes be derived from a FormJobs class?

Anyway, redesign without everyone causing to manipulate StartForm

Apparently there are a limited number of StartForm layouts depending on AppRoot, defaultDevice etc. You seem to be missing some "else" after your if, so this list might not be accurate. Still you will get the idea:

enum StartFormLayouts
{
    DefaultDevice0,
    AppRoot0,
    Other,        
}

// class that specifies the layout of any startform
class AppJobs
{
    private bool IsAppRoot0 
    {
        get{return Appjobs.AppRoot == null || Appjobs.AppRoot == "0";}
    }
    private bool IsDefaultDevice0
    {
        get{return Appjobs.DefaultDevice == null || Appjobs.DefaultDevice == "0";}
    }

    public StartFormLayoug GetPreferredLayout()
    {
         if (this.IsAppRoot0)
         {
             if (this.IsDefaultDevice)
             {
                  return StartFormLayout.DefaultDevice0;
             }
             else
                  return StartFormLayout.AppRoot0;
          }
          else
          {
              return StartFormLayout.Other;
          }
    }

    public bool ShouldAskDirectoryCreation()
    {
        return (!this.IsAppRoot0 && !Directory.Exists(AppRoot));
    }
}

Note that this class doesn't need StartForm, nor AppJobs. It could work with any class that wants to know if it should ask for DirectoryCreation. Since it also does not speak any language, even a Chinese StartForm could use it. After all, the StartForm is the only one who knows what language it speaks and what to do if a certain layout is requested.

Furthermore, did you notice that I used a double || to use a Boolean OR instead of a bitwise or?

And I use statements like if (a) instead of if(a=true) a C# Boolean is a real Boolean, in contradiction to Booleans in C and C++.

The class of all kinds of forms that should be able to be layout according to the requested layout contains the functions similar to your

It depends a bit of whether you decide to let it be a base class of StartForm or StartForm itself. If you want it to handle every form class that has the required controls, consider of using an interface:

public Interface IStartForm
{
    public int Height {get; set;}
    public Label LabelNoRoot {get;}
    public Label LabelNoDevice {get; }
    public Button BtnTickets {get;}
    ...

This way you can set the size of any form that has these labels and buttons, even if they have different names than those strings you use.

But again: if you ever only want to size StartForm, then this should be a function in StartForm.

public SetHeight(StartFormLayout layout, IStartForm startForm)
{
    switch (layout)
    {
        case StartFormLayout.DefaultDevice0:
            if (startForm.MenuStrip.Visible)
            {
                startForm.Height = ...;
                startForm.LabelNoRoot.Location = ...
                // etc
            }
            else
            {
               ...

Noticed that because of this separation of concerns the AppJobs and FormJobs don't have to know each other. AppJobs and FormJobs also don't really have to know what 'StartForm` is, only that it has the labels and buttons etc that it needs to change.

class StartForm : Form, IStartForm
{
    public Label LabelNoRoot {get{return this.label1; } }
    ...

    private void FrmStart_Load(object sender, EventArgs e)
    {
        AppJobs layoutdesigner = new AppJobs(...);
        StartFormLayout layoutdesigner = layouter.GetPreferredLayout();

        FormJobs layouter = new FormJobjs();
        layouter.SetHeight(this)
    }

Noticed that my form didn't have a label named "LabelNoRoot", but a Label1 that should function as a LabelNoRoot. Also: because I used types instead of string, You can be certain that I can't handle a label as if it was a button. I can't by accident try to press the label. Something that could easily been done when you were using strings to identify the items you want to layout.