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);
}
}
}
}
2 Answers 2
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;
}
}
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)} //....
}
-
\$\begingroup\$ thanks for your suggestion, what whole change would be made based on my code? \$\endgroup\$Mara– Mara2020年02月05日 07:23:45 +00:00Commented Feb 5, 2020 at 7:23