How Can I make my Program Faster?

[Cyclone]

Hi, I have some code for a button Mouse Down Event. When I click the button it takes about 30 seconds before it's code executes.

if (listBox1->SelectedIndex == 1)
	{
		if (checkBox163B_Song_ID == 1)
		{
			checkBox231->BackColor = Color::Green;
		}
               else checkBox231->BackColor = Color::White;
        }

There are 32 of these blocks and each contains ~200 if blocks that change the back colour of the ~200 check boxes depending on its id. since I added this code it takes way too long for the window to show up after I click the button.

My question is how can I make the code faster and more efficient.

Thanks for your time.

[coder777]

My suggestion would be: use struct and array (for fixed size) or std::vector.

Then you can probably change your code to 2 loops depending on what exactly the if's are doing.

[kbw]

You could try to collect as much information you can upfront, using a minimal amount of Windows messaging (that's probably where the time's spent), then process the result (strings/positions?) separately afterwards.

[George P]

If'n you are using Visual Studio, NOT VS Code, even the Community edition has a profiler available to track and log CPU and memory usage when running your app.

[Cyclone]

kbw and George P, I'm not sure what you mean.

coder777 I never used a struct and array or vector before. What does it do. and is there a tutorial on how too use it?

Thanks.

[George P]

If'n you are using Visual Studio probably the best tutorial is to have your code project loaded in VS and compiled, and start the performance profiler from the debug menu. (ALT+F2). That runs your app and gathers profile information.

The performance profiler in VS really is not that hard to figure out.

[Cyclone]

Yea, I tried it as you suggested but now what? I already know what code is slowing it down. All those 'ifs'... I tried a switch but that made no difference. And i'm not sure how to make a switch with the nested 'if' in the my example above. I will try to explain how my program is supposed to work.

I have a listbox that contains a list of music tracks. There are a bunch of checkboxes (quite a few) that are used as properties. I need to be able to highlight the checkbox depending on which song id it is(in the list box). So roughly 200 check boxes X 32 songs that is quite a few if's and else's...

Using vectors and struct are beyond my knowledge. I would have no idea where to start. :(

Is there some way to show a loading box or pre-load the code that is slowing things down? I tried showing a message box when the button is pressed but the message box won't show up until the slow code is done loading.

Thanks for your patience.

edit. I found this example of a vector... i'm trying to figure how to use it with my example.

#include <iostream>
#include <vector>
using namespace std;

int main() {
  vector<int> num {1, 2, 3, 4, 5};

  cout << "Initial Vector: ";

  for (const int& i : num) {
    cout << i << "  ";
  }

  // change elements at indexes 1 and 4
  num.at(1) = 9;
  num.at(4) = 7;

  cout << "\nUpdated Vector: ";

  for (const int& i : num) {
    cout << i << "  ";
  }

  return 0;
}

[kigar64551]

Do you really have 231 separate variables for your 231 checkboxes?

If so, that would be kind of insane. And, of course, it doesn't scale. Better start by learning how to use arrays!

Instead of something like:

CheckBox *checkBox000 = new CheckBox();
CheckBox *checkBox001 = new CheckBox();
CheckBox *checkBox002 = new CheckBox();
CheckBox *checkBox003 = new CheckBox();
/* ... 226 lines skipped ... */
CheckBox *checkBox231 = new CheckBox();

...you can write:

#define NUM_CHECKBOXES 231
CheckBox *checkBoxes[NUM_CHECKBOXES];
for (size_t i = 0; i < NUM_CHECKBOXES; ++i)
{
    checkBoxes[i] = new CheckBox();
}

Or using std::vector class:

#define NUM_CHECKBOXES 231
std::vector<CheckBox*> checkBoxes;
for (size_t i = 0; i < NUM_CHECKBOXES; ++i)
{
    checkBoxes.push_back(new CheckBox());
}

Then you can also do:

void updateBackColor(const int songId)
{
    for (size_t i = 0; i < NUM_CHECKBOXES; ++i)
    {
        checkBoxes[i]->BackColor = (checkBoxSongID[i] == songId) ? Color::Green : Color::White;
    }
}

...assuming that checkBoxSongID[] is also stored as an array now!

[Cyclone]

Ok thanks for your help. I think I understand your logic. I can work with that.

And yes I have all this variables lol. I knew that wasn’t the best aproach…

So I should create those checkboxes at runtime and use arrays…

In your last example how do I write it so that the backcolor turns green depending on the song ids. Without using all those ifs.

For example if song #1 is selected in the listbox and checkbox# 4 is checked. That means when when the user selects song#1 from the listbox the the checkbox when turn green. If not it will turn white. Make sense?

[kigar64551]

And yes I have all this variables lol. I knew that wasn’t the best aproach…

Whenever you need to store n items of something, where n may even vary, do not use n separate variables; instead use an array!

...or maybe an STL container like std::vector.

Note: The size n of an array can be chosen freely as needed, but is still fixed once the array was created.

An std::vector can grow/shrink dynamically, by adding or removing items.

In your last example how do I write it so that the backcolor turns green depending on the song ids. Without using all those ifs.

Exactly as was shown in the example?

Just use a loop to iterate all checkboxes, from #0 to #n-1, and update the color of each one, based on whatever condition you need.

for (size_t i = 0; i < NUM_CHECKBOXES; ++i)
{
    checkBoxes[i]->BackColor = (some_condition) ? Color::Green : Color::White;
}

...is a shorthand for:

for (size_t i = 0; i < NUM_CHECKBOXES; ++i)
{
    if (some_condition) checkBoxes[i]->BackColor = Color::Green;
    else checkBoxes[i]->BackColor = Color::White;
}

In the above code, replace "some_condition" with the actual condition you need! (e.g. checkBoxSongID[i] == 1)

BTW: Variables like checkBox163B_Song_ID should also be converted into a single array!

[Cyclone]

Oh I feel a little silly. I miss read that short hand example. Sorry!
It’s too bad I have to rewrite so much code. I like the designer cuzz I can see what’s going on. And all the control attributes are coded for me.

Thanks so much!

[kigar64551]

One of my most productive days was throwing away 1,000 lines of code.
— Ken Thompson (one of the creators of Unix and C)


Individually placing elements in the designer is nice as long as you only have a few (small/fixed number of) elements.

But it just won't scale, if you need n elements, where n is large, or even only known at runtime!

At that point, you are probably way better off by dynamically creating the elements at runtime, using a loop and an array or vector.

I think your code will be much shorter and much more readable/maintainable, after refactoring it 😄

[thmm]

There's no need to create an array or other container. All the controls are in an array(called Controls) in the Form class already. You can loop through this array, check if the control is a CheckBox and use the code from above code.
I assume you use C++ / CLR, though you didn't mention it.

[kigar64551]

Still, you really don't want to place like n = ~250 CheckBox'es individually in the designer 😬

Instead, you probably should be using a different type of UI control, such as a ListBox or ComboBox, which is intended to hold n items.

…or, if you really need such a huge number of separate CheckBox'es, better create them in "code behind" at runtime, by using a loop.

(this way you can also easily change n at any time, without having to mess with the designer)

[thmm]

@kigar64551,

Still, you really don't want to place like n = ~250 CheckBox'es individually in the designer
…or, if you really need such a huge number of separate CheckBox'es, better create them in "code behind" at runtime.

I agree with you, but the op has done it and I guess the op doesn't want to start from scratch again.

Instead, you probably should be using a different type of UI control, such as a ListBox or ComboBox, which is intended to hold n items.

I would recommend a CheckedListBox.

[kigar64551]

Also:
https://www.codeproject.com/Articles/1062442/A-Multi-Color-Multi-Font-CheckedListBox

[seeplus]

I don't know what your gui design looks like - but a common design is to have a treeview or listbox control on the left with a listview control on the right (similar to how Explorer looks). A lisview control has several different 'look's (details, icon etc).

[George P]

And if'n someone wants a WinAPI checked combobox or listbox that is Win32 C-based instead of WinForms/C#:
https://www.codeproject.com/Articles/5320735/Check-It-Out-2-0-One-Step-Win32-SDK-C-Checked-Comb

[Cyclone]

There's no need to create an array or other container. All the controls are in an array(called Controls) in the Form class already. You can loop through this array, check if the control is a CheckBox and use the code from above code.
I assume you use C++ / CLR, though you didn't mention it.

That is exactly what I want to do. What is the syntax? I don't want to start from scratch. Thanks.

I agree there other ways.. using different controls.

Edit..
This seems to work but how to detect Control Type?

private: System::Void button2_Click(System::Object^ sender, System::EventArgs^ e)
{
	for (int i = 0; i < 200; i++)
	{
		
			System::Windows::Forms::Control::Controls[i]->BackColor = Color::Green;
		
	}
}

[thmm]

@OP,
now you are on the right track.

for (int i = 0; i < Controls->Count; i++)
{
  CheckBox^ chk = dynamic_cast<CheckBox^>(Controls[i]);
  if (chk != nullptr)
  // chk->BackColor = Color::Green;
}

.NET makes many things very simple, but you need to learn certain idioms.
Also you need to forget as much standard C++ as possible.
Here's a good tutorial.
http://www.functionx.com/vccli/index.htm