Improving code functionality

Question

I use a change of groupbox locations depending on the number of weeks in the month. Is there any possibility for me to improve the functionality of this code a little bit, is it slow to load on this part? Thank you all for your help.

FormWindowState LastWindowState = FormWindowState.Minimized;
            private void Form1_Resize(object sender, EventArgs e)
            {
                // When window state changes
                if (WindowState != LastWindowState)
                {
                    LastWindowState = WindowState;

                    if (numberOfWeek == 4)
                    {
                        groupBoxFifthWeek.Hide();
                        groupBoxSixthWeek.Hide();
                        if (WindowState == FormWindowState.Maximized)
                        {
                            groupBoxFirstWeek.Location = new Point(1685, 140);
                            groupBoxSecondWeek.Location = new Point(1685, 325);
                            groupBoxThirdWeek.Location = new Point(1685, 510);
                            groupBoxFourthWeek.Location = new Point(1685, 700);
                        }
                        if (WindowState == FormWindowState.Normal)
                        {
                            groupBoxFirstWeek.Location = new Point(978, 105);
                            groupBoxSecondWeek.Location = new Point(978, 220);
                            groupBoxThirdWeek.Location = new Point(978, 345);
                            groupBoxFourthWeek.Location = new Point(978, 468);
                        }
                    }
                    else if (numberOfWeek == 5)
                    {
                        groupBoxFifthWeek.Show();
                        groupBoxSixthWeek.Hide();

                        if (WindowState == FormWindowState.Maximized)
                        {
                            groupBoxFirstWeek.Location = new Point(1685, 120);
                            groupBoxSecondWeek.Location = new Point(1685, 265);
                            groupBoxThirdWeek.Location = new Point(1685, 415);
                            groupBoxFourthWeek.Location = new Point(1685, 570);
                            groupBoxFifthWeek.Location = new Point(1685, 720);
                        }
                        if (WindowState == FormWindowState.Normal)
                        {
                            groupBoxFirstWeek.Location = new Point(978, 89);
                            groupBoxSecondWeek.Location = new Point(978, 187);
                            groupBoxThirdWeek.Location = new Point(978, 284);
                            groupBoxFourthWeek.Location = new Point(978, 383);
                            groupBoxFifthWeek.Location = new Point(978, 484);
                        }
                    }
                    else
                    {
                        groupBoxSixthWeek.Show();
                        if (WindowState == FormWindowState.Maximized)
                        {
                            groupBoxFirstWeek.Location = new Point(1685, 105);
                            groupBoxSecondWeek.Location = new Point(1685, 230);
                            groupBoxThirdWeek.Location = new Point(1685, 357);
                            groupBoxFourthWeek.Location = new Point(1685, 482);
                            groupBoxFifthWeek.Location = new Point(1685, 610);
                            groupBoxSixthWeek.Location = new Point(1685, 737);
                        }
                        if (WindowState == FormWindowState.Normal)
                        {
                            groupBoxFirstWeek.Location = new Point(978, 79);
                            groupBoxSecondWeek.Location = new Point(978, 162);
                            groupBoxThirdWeek.Location = new Point(978, 244);
                            groupBoxFourthWeek.Location = new Point(978, 328);
                            groupBoxFifthWeek.Location = new Point(978, 412);
                            groupBoxSixthWeek.Location = new Point(978, 496);
                        }
                    }
                }
            }

Xalbat Echeverria · Accepted Answer · 2020-01-30 14:52:27Z

I don't think there is much you can do in here performance-wise. However, you could improve the readability of your code with a few little things.

I'd personally use a switch statement instead of repeated else/if. It's simpler to read, we know right from the start that all conditions are based on "numberOfWeek".

Maybe there is something we could do repeated Maximized or Minimized conditions and groupBox locations assignments, but I don't really know right now.

Also, I'd return early if the WindowState is equal to LastWindowState. It makes code clearer, and we don't need to go all the way down your class to see that, in case WindowState hasn't changed, you do nothing.

            private void Form1_Resize(object sender, EventArgs e)
            {
                if (WindowState == LastWindowState)
                {
                    return;
                }

                LastWindowState = WindowState;

                switch (numberOfWeek)
                {
                    case 4:
                        groupBoxFifthWeek.Hide();
                        groupBoxSixthWeek.Hide();
                        if (WindowState == FormWindowState.Maximized)
                        {
                            groupBoxFirstWeek.Location = new Point(1685, 140);
                            groupBoxSecondWeek.Location = new Point(1685, 325);
                            groupBoxThirdWeek.Location = new Point(1685, 510);
                            groupBoxFourthWeek.Location = new Point(1685, 700);
                        }
                        if (WindowState == FormWindowState.Normal)
                        {
                            groupBoxFirstWeek.Location = new Point(978, 105);
                            groupBoxSecondWeek.Location = new Point(978, 220);
                            groupBoxThirdWeek.Location = new Point(978, 345);
                            groupBoxFourthWeek.Location = new Point(978, 468);
                        }
                        break;

                    case 5:
                        groupBoxFifthWeek.Show();
                        groupBoxSixthWeek.Hide();

                        if (WindowState == FormWindowState.Maximized)
                        {
                            groupBoxFirstWeek.Location = new Point(1685, 120);
                            groupBoxSecondWeek.Location = new Point(1685, 265);
                            groupBoxThirdWeek.Location = new Point(1685, 415);
                            groupBoxFourthWeek.Location = new Point(1685, 570);
                            groupBoxFifthWeek.Location = new Point(1685, 720);
                        }
                        if (WindowState == FormWindowState.Normal)
                        {
                            groupBoxFirstWeek.Location = new Point(978, 89);
                            groupBoxSecondWeek.Location = new Point(978, 187);
                            groupBoxThirdWeek.Location = new Point(978, 284);
                            groupBoxFourthWeek.Location = new Point(978, 383);
                            groupBoxFifthWeek.Location = new Point(978, 484);
                        }
                        break;

                    default:
                        groupBoxSixthWeek.Show();
                        if (WindowState == FormWindowState.Maximized)
                        {
                            groupBoxFirstWeek.Location = new Point(1685, 105);
                            groupBoxSecondWeek.Location = new Point(1685, 230);
                            groupBoxThirdWeek.Location = new Point(1685, 357);
                            groupBoxFourthWeek.Location = new Point(1685, 482);
                            groupBoxFifthWeek.Location = new Point(1685, 610);
                            groupBoxSixthWeek.Location = new Point(1685, 737);
                        }
                        if (WindowState == FormWindowState.Normal)
                        {
                            groupBoxFirstWeek.Location = new Point(978, 79);
                            groupBoxSecondWeek.Location = new Point(978, 162);
                            groupBoxThirdWeek.Location = new Point(978, 244);
                            groupBoxFourthWeek.Location = new Point(978, 328);
                            groupBoxFifthWeek.Location = new Point(978, 412);
                            groupBoxSixthWeek.Location = new Point(978, 496);
                        }
                        break;
                }
            }

steve · Accepted Answer · 2020-01-31 19:00:56Z

1

since in the suggested code you're creating new point object on each pass; you could create some static points in a collection and use them instead of creating new ones each time.

Example:

Static IDictionary<WindowState,IList<Point> points = new 
Dictionary<WindowState,IList<Point>(){
{ FormWindowState.Maximized, new List<Point>{
    {new Point(1685, 105)},
    {new Point(1685, 230)} //....
 }

answered Jan 31, 2020 at 19:00

steve

111 bronze badge

\$\begingroup\$ thanks for your suggestion, what whole change would be made based on my code? \$\endgroup\$

Mara
– Mara

2020-02-05 07:23:45 +00:00
Commented Feb 5, 2020 at 7:23

Add a comment |

Stack Exchange Network

Improving code functionality

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Improving code functionality

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions