Problem with Dynamic Creation of Components

[Alba Ra]

This is from a training project. So the objective is to learn how to do a specific task, though there may be better ways to do the same thing.

Okay, the background of the problem at hand:

It's from my training Mastermind clone. The user has chosen his colours and on mouse click those colours will appear in a separate form (as oval shapes from Microsoft.VisualBasic.PowerPacks) with two additional shapes, yet in specific distinct colours and a number telling the user how many well-placed slots he has found (and of course how many colours he has right as well, you know the game).

Each of these shapes will be created when the need arises.

Copying the user-chosen colours works fine:

for (int i = 0; i < GlobalVariables.configsetup[0]; i++)
{
    GlobalVariables.countresultslots += 1;
    ResultForm.ClientSize = new System.Drawing.Size(((GlobalVariables.configsetup[0] + 2) * 60) + 4, 362);
    ResultForm.Panel.Size = new System.Drawing.Size((GlobalVariables.configsetup[0] + 2) * 60, 362 - 24);
    ResultForm.SlotField[GlobalVariables.countresultslots] = new System.Windows.Forms.Panel();
    ResultForm.SlotContainer[GlobalVariables.countresultslots] = new Microsoft.VisualBasic.PowerPacks.ShapeContainer();
    ResultForm.SlotForm[GlobalVariables.countresultslots] = new Microsoft.VisualBasic.PowerPacks.OvalShape();
    ResultForm.Panel.Controls.Add(ResultForm.SlotField[GlobalVariables.countresultslots]);
    ResultForm.SlotField[GlobalVariables.countresultslots].Location = new Point(i * 60, (GlobalVariables.guesscounter - 1) * 60);
    ResultForm.SlotField[GlobalVariables.countresultslots].Controls.Add(ResultForm.SlotContainer[GlobalVariables.countresultslots]);
    ResultForm.SlotField[GlobalVariables.countresultslots].Margin = new System.Windows.Forms.Padding(5);
    ResultForm.SlotField[GlobalVariables.countresultslots].Name = "SlotField" + (GlobalVariables.countresultslots + 1).ToString();
    ResultForm.SlotField[GlobalVariables.countresultslots].Size = new System.Drawing.Size(50, 50);
    ResultForm.SlotField[GlobalVariables.countresultslots].TabIndex = 0;
    ResultForm.SlotField[GlobalVariables.countresultslots].TabStop = false;
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Location = new System.Drawing.Point(0, 0);
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Margin = new System.Windows.Forms.Padding(0);
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Name = "SlotContainer" + (GlobalVariables.countresultslots + 1).ToString();
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Shapes.AddRange(new Microsoft.VisualBasic.PowerPacks.Shape[] { ResultForm.SlotForm[GlobalVariables.countresultslots] });
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Size = new System.Drawing.Size(50, 50);
    ResultForm.SlotContainer[GlobalVariables.countresultslots].TabIndex = 0;
    ResultForm.SlotContainer[GlobalVariables.countresultslots].TabStop = false;
    ResultForm.SlotForm[GlobalVariables.countresultslots].BorderColor = System.Drawing.Color.Black;
    ResultForm.SlotForm[GlobalVariables.countresultslots].BorderWidth = 2;
    ResultForm.SlotForm[GlobalVariables.countresultslots].Enabled = false;
    ResultForm.SlotForm[GlobalVariables.countresultslots].FillColor = System.Drawing.Color.FromArgb(BasicFunctions.SetColour(GlobalVariables.tempcolorinput[i])[0], BasicFunctions.SetColour(GlobalVariables.tempcolorinput[i])[1], BasicFunctions.SetColour(GlobalVariables.tempcolorinput[i])[2]);
    ResultForm.SlotForm[GlobalVariables.countresultslots].FillStyle = Microsoft.VisualBasic.PowerPacks.FillStyle.Solid;
    ResultForm.SlotForm[GlobalVariables.countresultslots].Location = new System.Drawing.Point(5, 5);
    ResultForm.SlotForm[GlobalVariables.countresultslots].Name = "SlotForm" + (GlobalVariables.countresultslots + 1).ToString();
    ResultForm.SlotForm[GlobalVariables.countresultslots].Size = new System.Drawing.Size(38, 38);
}

...where GlobalVariables.configsetup[0] stocks the number of colours (let's say 4), GlobalVariables.countresultslots will count the number of shapes created and thus help identify each shape, GlobalVariables.tempcolorinput has temporarily stocked the colours chosen by the user, and BasicFunctions.SetColour is a function that will provide the RGB codes for each color.

Up until now the code does exactly what I wanted it to do. Now I created a second loop right after the other to create two more shapes with a number in them, set in different colours, but right next to the others.

for (int i = 0; i < 2; i++)
{
    GlobalVariables.countresultslots += 1;
    ResultForm.SlotField[GlobalVariables.countresultslots] = new System.Windows.Forms.Panel();
    ResultForm.SlotContainer[GlobalVariables.countresultslots] = new Microsoft.VisualBasic.PowerPacks.ShapeContainer();
    ResultForm.SlotForm[GlobalVariables.countresultslots] = new Microsoft.VisualBasic.PowerPacks.OvalShape();
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i] = new System.Windows.Forms.Label();
    ResultForm.Panel.Controls.Add(ResultForm.SlotField[GlobalVariables.countresultslots]);
    ResultForm.Panel.Controls.Add(ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i]);
    ResultForm.SlotField[GlobalVariables.countresultslots].Location = new Point((GlobalVariables.configsetup[0] + i)* 60, (GlobalVariables.guesscounter - 1) * 60 + 20);
    ResultForm.SlotField[GlobalVariables.countresultslots].Controls.Add(ResultForm.SlotContainer[i]);
    ResultForm.SlotField[GlobalVariables.countresultslots].Margin = new System.Windows.Forms.Padding(5);
    ResultForm.SlotField[GlobalVariables.countresultslots].Name = "SlotField" + (GlobalVariables.countresultslots + 1).ToString();
    ResultForm.SlotField[GlobalVariables.countresultslots].Size = new System.Drawing.Size(50, 50);
    ResultForm.SlotField[GlobalVariables.countresultslots].TabIndex = 0;
    ResultForm.SlotField[GlobalVariables.countresultslots].TabStop = false;
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Location = new System.Drawing.Point(0, 0);
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Margin = new System.Windows.Forms.Padding(0);
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Name = "SlotContainer" + (GlobalVariables.countresultslots + 1).ToString();
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Shapes.AddRange(new Microsoft.VisualBasic.PowerPacks.Shape[] { ResultForm.SlotForm[GlobalVariables.countresultslots] });
    ResultForm.SlotContainer[GlobalVariables.countresultslots].Size = new System.Drawing.Size(50, 50);
    ResultForm.SlotContainer[GlobalVariables.countresultslots].TabIndex = 0;
    ResultForm.SlotContainer[GlobalVariables.countresultslots].TabStop = false;
    ResultForm.SlotForm[GlobalVariables.countresultslots].BorderWidth = 2;
    ResultForm.SlotForm[GlobalVariables.countresultslots].Enabled = false;
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].AutoSize = true;
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Location = new Point((GlobalVariables.configsetup[0] + i)* 60, (GlobalVariables.guesscounter - 1) * 60);
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Size = new System.Drawing.Size(42, 20);
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].TabIndex = 0;
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Enabled = false;
    if (i == 0)
    {
        ResultForm.SlotForm[GlobalVariables.countresultslots].FillColor = System.Drawing.Color.Green;
        ResultForm.SlotForm[GlobalVariables.countresultslots].BorderColor = System.Drawing.Color.DarkGreen;
        ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Name = "CH" + GlobalVariables.guesscounter.ToString() + "." + (i + 1).ToString();
        ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Text = GlobalVariables.position[GlobalVariables.guesscounter, GlobalVariables.configsetup[0]].ToString();
    }
    else
    {
        ResultForm.SlotForm[GlobalVariables.countresultslots].FillColor = System.Drawing.Color.Green;
        ResultForm.SlotForm[GlobalVariables.countresultslots].BorderColor = System.Drawing.Color.DarkGreen;
        ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Name = "CC" + GlobalVariables.guesscounter.ToString() + "." + (i + 1).ToString();
        ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Text = GlobalVariables.position[GlobalVariables.guesscounter, GlobalVariables.configsetup[0] + 1].ToString();
    }
    ResultForm.SlotForm[GlobalVariables.countresultslots].FillStyle = Microsoft.VisualBasic.PowerPacks.FillStyle.Solid;
    ResultForm.SlotForm[GlobalVariables.countresultslots].Location = new System.Drawing.Point(5, 5);
    ResultForm.SlotForm[GlobalVariables.countresultslots].Name = "SlotForm" + (GlobalVariables.countresultslots + 1).ToString();
    ResultForm.SlotForm[GlobalVariables.countresultslots].Size = new System.Drawing.Size(38, 38);
}

Theoretically it should work, but a funny thing happens. Once the program is finished, it should display 4 shapes with the colours the user has chosen (4 because we have agreed on this much, remember) and two to the right in a different colour and a number in them.

But what I actually get is that the first two shapes are missing, shapes 3 and 4 are displayed alright, and the the last two shapes (that should have been in standard Green, in my code so far) will display the colours of shapes 1 and 2, respectively. And I can't think of any reason why this should be the case.

[DdoubleD]

Alba, I don't really know this game, but I did spot a couple of things I thought worth mentioning. Have you stepped through each line of this code to see if what is happening?

I noticed you are incrementing GlobalVariables.countresultslots at the beginning of each loop, and then referencing an element of ResultForm.SlotField[GlobalVariables.countresultslots] from that location. On the surface, this looks very suspicious because usually array/list elements are not referenced higher than Count - 1 .

You also calculate an indexer several times for the same purpose of referencing an element: [(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i] . Not only is this redundant and a waste of processing time, but it makes it more difficult to watch what is happening. I suggest breaking the calculation out, for example:

// change
ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].AutoSize = true;
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Location = new Point((GlobalVariables.configsetup[0] + i)* 60, (GlobalVariables.guesscounter - 1) * 60);
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Size = new System.Drawing.Size(42, 20);
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].TabIndex = 0;
    ResultForm.ResultLabel[(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i].Enabled = false;

// to something like:
            int index = (GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i;
            ResultForm.ResultLabel[index].AutoSize = true;
            ResultForm.ResultLabel[index].Location = new Point((GlobalVariables.configsetup[0] + i) * 60, (GlobalVariables.guesscounter - 1) * 60);
            ResultForm.ResultLabel[index].Size = new System.Drawing.Size(42, 20);
            ResultForm.ResultLabel[index].TabIndex = 0;
            ResultForm.ResultLabel[index].Enabled = false;

That also makes it easier to read and there won't be a need to create a temporary storage to hold the result of the calc every time you use it.

Aside from that, I don't think I can determine why your program does not produce the desired result without knowing more about your program and its code.

[sknake]

I am with DdoubleD on needing to see more code. Could you upload a sample project demonstrating the project?

[Geekitygeek]

You beat me to it DdoubleD :p
I was gonna point out those same points.
Only other point i had was to check that GlobalVariables.countresultslots is set correctly at the start of the loop...since your using it to reference an array and incrementing it at the start of the loop it should be set to -1 before the loop starts. That way the first increment will put it to 0 for the first element in array.

Also, i was confused by the calculation DdoubleD pointed out...why are you calculating the array element this way?
Reading through it, and assuming that GlobalVariables.countresultslots starts at -1 or 0 then GlobalVariables.countresultslots will never be larger than (GlobalVariables.configsetup[0] + 2)) .
Since you are performing an integer division, the result of the division will be truncated and result in 0 + i.

For example:

If configsetup = 4 and counterresultslots = -1

After your first loop counterresultslots = 3.

in the second loop you then have:
counterresultslots/configsetup+2 == 4/6 == 0

[Alba Ra]

Okay, let me answer each point, one after the other.

With GlobalVariables.countresultslots I count the total number of slots needed to display. And yes, I forgot to mention, it starts at -1 so before the first slot form is created, it will be 0 (as it should to address an array like ResultForm.SlotField[]).

As for calculating [(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i], I felt (for some reason I can not remember) that I should as little variables as possible. I'll change that to improve readability and processing.

And the result shall be trunctated, as only for two slot forms ResultLabels have to be created. So if GlobalVariables.configsetup[0] is 4, only a total of six slots will have been created. The calculation thus is for (GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i => (0/(4+2)) + i = 0 + i through (5/(4+2)) + i = 0 + i. As in for each GlobalVariables.configsetup[0] + 2 only 2 will be created. That's the origin of the 2 in the first place.

Where I don't follow Ryshad is why you think that GlobalVariables.countresultslots would be at 3 after one loop? It is incremented once each loop (at the start) by exactly 1.

Besides, the code may be hard to read, but it is working. It is adding the second loop, not inside but after the first loop, that creates the problem.

Still, my first step will be implementing those variables to ease the pain a bit... :icon_wink:

As for the game, it's a training clone of Mastermind.

[Geekitygeek]

I meant after ALL iterations of the first loop:
If it starts at -1 and is incremeneted by one in each iteration then:
Start : .countresultslots = -1
End of iteration 1(i=0) : .countresultslots= 0
End of iteration 2(i=1) : .countresultslots= 1
End of iteration 3(i=2) : .countresultslots = 2
End of iteration 4(i=3) : .countresultslots= 3

(GlobalVariables.countresultslots / (GlobalVariables.configsetup[0] + 2)) + i => (0/(4+2)) + i = 0 + i through (5/(4+2)) + i = 0 + i.

Even if you were calculating 0/(4+2) => 5/(4+2) the result is still going to be 0 so you are still calculating 0 + i each time. You only want 2 labels made, but you handled that by only having 2 iterations of the second loop. You can just use 'i' here and avoid the calculation all together :)

As for using fewer variables, storing the value once in a variable and then calling it each time its needed is far more efficient than recalculating it each time you need it. But you really dont need it.

[Alba Ra]

I am sorry, but it is necessary to do the calculation, as this is to display the colours the user has chosen each round including his previous choices.

If you don't know the game: the program will create a random combination of colours filling GlobalVariables.configsetup[0] slots, that the player has to figure out. Each round the program will tell to what degree the player has guessed right, by telling him how many slots he guessed with the exact colour and how many colours he guessed right but not in the correct slot. The program will, of course, not tell him which ones are correct.

So to help the player, each guess with the hint the program provides will be stored. And that's what this code is all about.

Each round the program will copy the colours of GlobalVariables.configsetup[0] slots to the new form/window, where GlobalVariables.configsetup[0] slots will be created. Plus two additional slots in neutral colours with a label telling the player the correct slots and correct colours. Thus each round, GlobalVariables.configsetup[0] + 2 slots will have to be created anew. That's why this calculation is necessary to create only 2 labels for every GlobalVariables.configsetup[0] + 2 slots.

I know that the code isn't complete yet, but it should work for the first round. And as I said, as long I only use the code to create the first GlobalVariables.configsetup[0] slots, it is working alright. But once I create those two additional slots, they will remove the first two slots and take their properties instead of their own.

Besides, the calculation (that I have already replaced by a variable called int tempindex) will only come into place the the last two slots when GlobalVariables.countresultslots will reach 4 and 5 (for each respective loop), thus creating ResultLabel[0] and ResultLabel[1].

But the next round with the calculation, ResultLabel[2] and ResultLabel[3] will be created, so that each round it will increment too (as it should).

I'm now trying to circumvent this problem by creating an algorithm that will allow everything to happen in just one loop. But as long as I don't understand why SlotForm[4] and SlotForm[5] take over the properties of SlotForm[0] and SlotForm[1] and delete those, I don't know how to recode is program without creating the same mistake.

[Geekitygeek]

My apologies, you didnt mention that the counter is persistent. I was assuming it is initialised to -1 EVERY time the two loops are run. My bad, in light of that, the calculation makes sense :)
Moving it to a variable is a good way to go though.

As for the problem of the slots displaying wrongly..there's nothing obvious in your code to explain it so if you post your project we can take a closer look for you.

[Alba Ra]

Okay, found the problem, some stupid mix-up.

In my first message I entered two codes, and the error was in the second one, line 11 which reads

[B]ResultForm.SlotField[GlobalVariables.countresultslots].Controls.Add(ResultForm.SlotContainer[i]);[/B]

but should have been

[B]ResultForm.SlotField[GlobalVariables.countresultslots].Controls.Add(ResultForm.SlotContainer[GlobalVariables.countresultslots]);[/B]

.
I found out about it when I decided to change (for readability) the incrementing variable in the second loop from i to j.

So, basically my code was doing its job, it didn't do mine. :$

Thanks for your help!!!

A special kudos to Ryshad!

[Geekitygeek]

Thank..not sure what i did to deserve it, but thanks anyways :)

remeber to mark the thread as solved