I have an application built in C# and Windows Forms. I have a basic panel open with several UserControls using different buttons.

For example, I have the PC button and Laptop and Tablet; for each button I created 2 UserControls, one occupying the (dock properties) top of the container and the other the center (dock fill).

The problem lies in, I have a panelContainer which is the main panel to show something. When I press the Home button, I want it to open the 2 UserControls and to hide the others, the same should be the case with the other buttons.

Each UserControl created occupies that panelContainer, but I can't hide the other UserControls.

This is the cod creating the panelContainer to display the UserControl:

static Form1 _obj;

    public static Form1 Instance
    {
        get
        {
            if (_obj == null)
            {
                _obj = new Form1();
            }
            return _obj;
        }
    }

    public Panel PnlContainer
    {
        get { return panelContainer; }
        set { panelContainer = value; }
    }

This is the Button PC which open that 2 userControl in panelContainer

private void guna2Button4_Click(object sender, EventArgs e)
{
        // here the 2 userControls are created and added to the panelContainer
        if (!Form1.Instance.PnlContainer.Controls.ContainsKey("UserControlSistemePC"))
        {
            UserControlSistemePC pc = new UserControlSistemePC();
            pc.Dock = DockStyle.Fill;
            Form1.Instance.PnlContainer.Controls.Add(pc);
            UserControlPCsiPeriferice_Menu pcMenu = new UserControlPCsiPeriferice_Menu();
            pcMenu.Dock = DockStyle.Top;
            Form1.Instance.PnlContainer.Controls.Add(pcMenu);
        }
        Form1.Instance.PnlContainer.Controls["UserControlPCsiPeriferice_Menu"].BringToFront();
        Form1.Instance.PnlContainer.Controls["UserControlSistemePC"].BringToFront();
        Form1.Instance.BackButton.Visible = true;
       
    }

This is the Button Laptop which open that 2 userControl in panelContainer

private void buttonLaptop_Click(object sender, EventArgs e)
{
        // here the 2 userControls are created and added to the panelContainer
        if (!Form1.Instance.PnlContainer.Controls.ContainsKey("UserControlLaptop"))
        {
            UserControlLaptop laptop = new UserControlLaptop();
            laptop.Dock = DockStyle.Fill;
            Form1.Instance.PnlContainer.Controls.Add(laptop);
            UserControlLaptop_Menu laptopMenu = new UserControlLaptop_Menu();
            laptopMenu.Dock = DockStyle.Top;
            Form1.Instance.PnlContainer.Controls.Add(laptopMenu);
        }
        Form1.Instance.PnlContainer.Controls["UserControlLaptop_Menu"].BringToFront();
        Form1.Instance.PnlContainer.Controls["UserControlLaptop"].BringToFront();
        Form1.Instance.BackButton.Visible = true;
    }

This is the Button Tablet which open that 2 userControl in panelContainer

private void buttonTablete_Click(object sender, EventArgs e)
{
        // here the 2 userControls are created and added to the panelContainer
        if (!Form1.Instance.PnlContainer.Controls.ContainsKey("UserControlTablete"))
        {
            UserControlTablete tablete = new UserControlTablete();
            tablete.Dock = DockStyle.Fill;
            Form1.Instance.PnlContainer.Controls.Add(tablete);
            UserControlTablete_Menu tableteMenu = new UserControlTablete_Menu();
            tableteMenu.Dock = DockStyle.Top;
            Form1.Instance.PnlContainer.Controls.Add(tableteMenu);
        }
        Form1.Instance.PnlContainer.Controls["UserControlTablete_Menu"].BringToFront();
        Form1.Instance.PnlContainer.Controls["UserControlTablete"].BringToFront();
        Form1.Instance.BackButton.Visible = true;

    }

As you can see, each individual button opens the 2 USerControls and displays them, but my problem is how to hide the others. I have tried these 2 methods, but none of them work, and I have the same error with both.

Form1.Instance.PnlContainer.Controls["UserControlTablete"].Hide();//.Visible = false
panelContainer.Controls["UserControlSistemePC"].Hide();

And the error is this:

System.NullReferenceException: 'Object reference not set to an instance of an object.'

How can I hide them ? Please help.

CodePudding user response：

Firstly, apologies in advance for the length of the response, and additionally for taking this long to reply. I'd been meaning to tackle the question, but for that I needed VS, and I was working on a project in dual-booted Linux so I didn't want to reboot into Windows...you know how that goes.

Secondly, apologies in advance for the plethora of critiques I'm about to subject your work to.

Alright, with that out of the way let's start with the question. I was confused why anyone would downvote it, as at first blush it seemed fairly well spelled-out complete with code samples and everything. I now suspect the reasons had to do with the fact that the code blocks weren't perfectly formatted (for example the first line of your first sample is indented a tab less than everything that follows) in addition to the lack of clarity regarding whether all the code was in the same class or what. Just so we're on the same page, I assumed all of the samples you gave to be in the Form1 class, which in my reconstruction contains nothing else except the default constructor and the partial half of the class that's autogenerated. My designer looks as follows: In the top-left is the Panel called panelContainer. In your post you refer to "a panelContainer" implying that's the name of some custom class or something, however from the code snippet I inferred it's just a normal Panel. Also, in order for it to compile I had to create six other control types. Since I'm lazy, I just ripped off Button:

namespace StackOverflowWinformsTest
{
    internal class UserControlLaptop : Button
    {
        public UserControlLaptop() : base()
        {
            Text = nameof(UserControlLaptop);
            Name = nameof(UserControlLaptop);
        }
    }
}

The others are exactly the same, just with different names.

From what I understand, you ultimately would like to simply add the two hiding lines you post at the end of each of the click handlers.

Ok, here's my first (albeit minor) criticism: a couple of your names leave a lot to be desired. _obj is about as undescriptive as you can get. Personally I'm not a fan of the underscore convention for private properties, although I know some people and groups like it. But please, at least call it "_mainForm" or something. Since you have a getter-setter right there, "instance" presents itself as a decent go-to option. The other name that caught my attention was PnlContainer. Hungarian notation is fairly widely discouraged in modern object-oriented languages, but I suspect this is just short for "PanelContainer". In my opinion you could've just had the full name, or otherwise just called it "Container". Oh, and finally "guna2Button4_Click"? I'm not even sure where to start with this one...I know this point wasn't part of the question, but since I'm doing a thorough commentary on the code anyway I figured I'd just add it in for good measure.

In my first test to reproduce your issue, I found that the line Form1.Instance.PnlContainer.Controls["UserControlPCsiPeriferice_Menu"].BringToFront(); already causes a null-reference exception by virtue of the fact that ControlCollection[string] returns a value based on the Name of the Control, which isn't set in your code. I presume from context it's set in the controls themselves, which is why I added the line Name = nameof(UserControlLaptop); in my test controls.

Form1.Instance.PnlContainer.Controls["UserControlTablete"].Hide();

Causes a NullReferenceException because ...Controls["UserControlTablete"] is null the first time the code is executed, so you're essentially calling null.Hide(); The root cause is that no control with the name "UserControlTablete" has been added yet to Instance.PnlContainer. There are a couple ways to fix this:

// Just check if the key exists, and if not don't do anything
if (Form1.Instance.PnlContainer.Controls.ContainsKey("UserControlTablete"))
    Form1.Instance.PnlContainer.Controls["UserControlTablete"].Hide();
// Or, better yet, use the null-conditional operator
Form1.Instance.PnlContainer.Controls["UserControlSistemePC"]?.Hide();

The Control.Hide() method works fine if it's called using one of the ways listed above, but in your questioned you mentioned trying panelContainer.Controls["UserControlSistemePC"].Hide(); as well. In short, this:

panelContainer.Controls["UserControlSistemePC"]?.Hide();

will work, however I'm not sure the overall behavior here is what you're looking for.

Essentially, the issue is that panelContainer and Form1.Instance.PnlContainer may not be the same object, and if I had to guess I would say they aren't.

The declaration

public static Form1 Instance {get {...}}

essentially makes it so that there is just one instance of Form1 that you access by calling Form1.Instance. However, there can be other instances of Form1, and chances are when your program starts it does so with a line that reads:

Application.Run(new Form1());

If that's the case, when you press run you will see a Form1 that is different from what's returned by Form1.Instance. In fact, with the Form1 class written the way it is, if you wanted to start the program with the same instance as you get when you call Form1.Instance you would need to do

Application.Run(Form1.Instance);

If that's how you're using the class, then functionally it makes no difference whether you write panelContainer or Form1.Instance.PnlContainer.

However, consider this (development philosophy alert!!!): Normally when designing a class, it shouldn't matter how the class is used.

If you've designed the class to internally call methods on a static instance of itself on the assumption that that static instance is the same as the current instance, you can be sure that at some point someone's going to want to create another instance and find it doesn't work at all like expected.

Honestly, from what I've seen I'm not sure there's any reason for the Instance property to exist at all. All the calls to Form1.Instance.whatever can just be changed to whatever. If you do this then, is there any reason you will still need the PnlContainer property? If not, you could get rid of that too, and just use panelContainer directly instead.

Here's one last point about calling static properties and methods from inside the class where they're defined. You actually don't even need the class name, so

Form1.Instance.PnlContainer.Controls["UserControlPCsiPeriferice_Menu"].BringToFront();
Form1.Instance.PnlContainer.Controls["UserControlSistemePC"].BringToFront();
Form1.Instance.BackButton.Visible = true;
Form1.Instance.PnlContainer.Controls["UserControlTablete"]?.Hide();
Form1.Instance.PnlContainer.Controls["UserControlSistemePC"]?.Hide();

is exactly the same as

Instance.PnlContainer.Controls["UserControlPCsiPeriferice_Menu"].BringToFront();
Instance.PnlContainer.Controls["UserControlSistemePC"].BringToFront();
Instance.BackButton.Visible = true;
Instance.PnlContainer.Controls["UserControlTablete"]?.Hide();
Instance.PnlContainer.Controls["UserControlSistemePC"]?.Hide();

That brings us to the last form-related point I wanted to bring up: please, please don't write code that requires hardcoded string values for functionality if there's any other way (which there normally is, especially in modern languages like C#). Sooner or later you'll end up tracking down endless bugs caused by if (status == "deactivated") instead of if (status == "Deactivated") or having to rename something and not being sure where all the hardcoded strings are.

In this case, an easy drop-in replacement for

Form1.Instance.PnlContainer.Controls["UserControlTablete"]?.Hide();

would be

Form1.Instance.PnlContainer.Controls.OfType<UserControlTablete>().SingleOrDefault().Hide();

I chose the SingleOrDefault() method rather than First() because that way if there are multiple UserControlTabletes, it will throw an exception rather than just hiding the issue.

An alternative which I suspect would have slightly better performance would be to have a private variable for each of those controls we'll want to hide:

private UserControlSistemePC sistemePC;
private UserControlLaptop laptop;
private UserControlTablete tablete;

And then the if statements in the click handlers become

if (sistemePC == null)
{
    // Do stuff;
}

Finally, the hide statements simply look like this:

laptop?.Hide();
tablete?.Hide();

I'm going to conclude with a refactored version of the Form1 class removing the Instance property and getting rid of the access-by-string:

namespace StackOverflowWinformsTest
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        private UserControlSistemePC sistemePC;
        private UserControlPCsiPeriferice_Menu sistemePCMenu;
        private UserControlLaptop laptop;
        private UserControlLaptop_Menu laptopMenu;
        private UserControlTablete tablete;
        private UserControlTablete_Menu tableteMenu;

        private void sistemePC_Click(object sender, EventArgs e)
        {
            if (sistemePC == null)
            {
                sistemePC = new UserControlSistemePC();
                sistemePC.Dock = DockStyle.Fill;
                panelContainer.Controls.Add(sistemePC);
                sistemePCMenu = new UserControlPCsiPeriferice_Menu();
                sistemePCMenu.Dock = DockStyle.Top;
                panelContainer.Controls.Add(sistemePCMenu);
            }
            sistemePCMenu.BringToFront();
            sistemePC.BringToFront();
            BackButton.Show();
            laptop?.Hide();
            tablete?.Hide();
        }

        private void buttonLaptop_Click(object sender, EventArgs e)
        {
            if (laptop == null)
            {
                laptop = new UserControlLaptop();
                laptop.Dock = DockStyle.Fill;
                panelContainer.Controls.Add(laptop);
                laptopMenu = new UserControlLaptop_Menu();
                laptopMenu.Dock = DockStyle.Top;
                panelContainer.Controls.Add(laptopMenu);
            }
            laptopMenu.BringToFront();
            laptop.BringToFront();
            BackButton.Show();
            tablete?.Hide();
            sistemePC?.Hide();
        }

        private void buttonTablete_Click(object sender, EventArgs e)
        {
            if (tablete == null)
            {
                tablete = new UserControlTablete();
                tablete.Dock = DockStyle.Fill;
                panelContainer.Controls.Add(tablete);
                tableteMenu = new UserControlTablete_Menu();
                tableteMenu.Dock = DockStyle.Top;
                panelContainer.Controls.Add(tableteMenu);
            }
            tableteMenu.BringToFront();
            tablete.BringToFront();
            BackButton.Show();
            laptop?.Hide();
            sistemePC?.Hide();
        }
    }
}

If you have a lot of similar controls you might consider developing a more complete framework for them so you don't end up with a bunch of handlers with virtually the same stuff, but as-is it's probably fine.

Note that if you're using the most recent version of C#, you'll get warnings about fields potentially being null. You'll just need to update the syntax to fix those by marking them as nullable in the definition:

private UserControlSistemePC? sistemePC;

(While you're at it you can simplify the new statements too.)

Finally, you might also bear in mind the possibility of using Lazy<T> for your lazy evaluation needs.

I hope you've found at least some of this helpful, and let me know if you have any followup questions or if I wasn't clear on something.