Tech Off Thread

6 posts

Forum Read Only

This forum has been made read only by the site admins. No new threads or comments can be added.

Replacing Object Target in For Loops (C#)

Back to Forum: Tech Off
  • User profile image
    Human​Resources

    Hi there folks,

    I'm working on a World of Warcraft loot distributor and have the bulk of the code laid out.

    The majority of the actual programming is done but now as I go back to clean up the code I see a glaring issue.

    The button click event that I'm using is a 1600+ Line if statement...

    This is a problem.

    I thought that I would solve the issue by using a For loop however because of the amount of information compared in one Button click I can't outright add a For loop.

    Here's a bit of sample code.

     private void button2_Click(object sender, EventArgs e)
                {
                    if (l1tag > 0)
                    {
                       if (l1tag == 1)
                                {
                                    if (mailarmor.Checked || radioButton1.Checked || platearmor.Checked || onemaces.Checked || oneaxes.Checked || oneswords.Checked || twomaces.Checked || twoswords.Checked || twoaxes.Checked || polearms.Checked)
                                    {
                                        rannum = randNum.Next(101);
                                        textBox1.Text = rannum.ToString();
                                        i++;
                                    }
                                    else
                                    {
                                        textBox1.Text = "Not Eligible for Roll";
                                        i++;
                                    }
                                }
                                else if (l1tag == 2)
                                {
                                    if (clotharmor.Checked || radioButton1.Checked || leatherarmor.Checked || onemaces.Checked || twomaces.Checked || polearms.Checked || staves.Checked || fistweapons.Checked || daggers.Checked)
                                    {
                                        rannum = randNum.Next(101);
                                        textBox1.Text = rannum.ToString();
                                        i++;
                                    }
                                    else
                                    {
                                        textBox1.Text = "Not Eligible for Roll";
                                        i++;
                                    }
                                }
                                else if (l1tag == 3)
                                {
                                    if (mailarmor.Checked || radioButton1.Checked || onemaces.Checked || twomaces.Checked || oneaxes.Checked || twoaxes.Checked || staves.Checked || fistweapons.Checked || daggers.Checked)
                                    {
                                        rannum = randNum.Next(101);
                                        textBox1.Text = rannum.ToString();
                                        i++;
                                    }
                                    else

    The above code continues until l1tag = 10 at which point a blank is drawn.  The program then moves to  textbox2 and starts to figure out what to include in said textbox...

    Unfortunately this means the code is repeated again and again...

    TL;DR

    What I basically need to do is make it so that as i is incremented, the textbox and variable l1tag also increases in value.  (For instance if i = 2 then textBox2.Text and l2tag need to be used)

    Are there any simple ways in which I can accomplish this?  Thanks in advance!  Smiley

  • User profile image
    Frank Hileman

    Create a private class or structure called Condition, containing a public readonly IEnumerable<Control> or array member called Controls, and a public readonly integer called Tag.  Create a Condition constructor initializing the members. Initialize an array or list of Conditions with the set of controls and tags you have listed in your code. Then you can iterate over the Conditions, and iterate over each Control in a Condition, if the Tag matches.

  • User profile image
    Cupiditas

    bool allChecked;
    
    if (l1tag == 1) allChecked = ...;
    else if (l2tag == 2) allChecked = ...;
    // continued...
    
    if (allChecked)
    {
        rannum = randNum.Next(101);
        textBox1 = rannum.ToString();
        i++;
    }
    else
    {
        textBox1.Text = "Not Eligible for Roll";
        i++;
    }

    ?

    Edit: Frank has a way better solution, it depends how seriously you want to take the architecture.

  • User profile image
    Human​Resources

    Those are actually great solutions, thank you so much for posting.

    I went ahead and racked my ammature programming brain and came up with something very simple... more if statements.

    First I assign my l#tag values to a common variable, in this case, simply called "tag":

    private void button2_Click(object sender, EventArgs e)
                {
                    string textinfo;
                    int tag =0;
                    if (l1tag > 0)
                    {
                        for (int i = 1; i < 11 && i > 0; )
                        {
                            if (i == 1)
                            {
                                tag = l1tag;
                            }
                            else if (i == 2)
                            {
                                tag = l2tag;
                            }
                            else if (i == 3)
                            {
                                tag = l3tag;
                            }

    Then I go through one itteration of the code presented in the first post only instead of assigning values to each individual textbox it assigns the value to a string variable called textinfo.

    After the program assigns the value, it goes through one more if statement.

    if (i == 1)
                            {
                                textBox1.Text = textinfo.ToString();
                                i++;
                            }
                            else if (i == 2)
                            {
                                textBox2.Text = textinfo.ToString();
                                i++;
                            }
                            else if (i == 3)
                            {
                                textBox3.Text = textinfo.ToString();
                                i++;
                            }

     

    This then takes the assigned value and puts it in a textbox depending on the value of i.

    In the "assigning" if statement, there is information inside to clear past entries so duplicates don't appear...

    Guess I kinda answered my own question, but your answers are FAR more optimal.

    Thanks for the help guys! Smiley

  • User profile image
    figuerres

    some of that IF / ELSE might be better done with

    switch ( ){

    case 1: { stuff}; break;

    case 2: {stuff}; break;

    }

     

  • User profile image
    ScanIAm

    Create a List<> of objects where each object stores all the data needed to do 1 iteration of the loop.  Then you'd iterate through the list of objects.

     

Conversation locked

This conversation has been locked by the site admins. No new comments can be made.