C# If loop within a for loop within an if loop

Question

I would like to reduce this code if I can:

class Alarm
{
    internal static void isGreaterThanOrBelowValue(int min, int max, int now, int i)
    {
        MainWindow mw = new MainWindow();

        if (now < min && now !=0)
        {
            if(i == 1)
            {
                mw.TxtBox1.Foreground = new SolidColorBrush(Colors.Red);
            }
            if (i == 2)
            {
                mw.TxtBox2.Foreground = new SolidColorBrush(Colors.Red);
            }
            if (i == 3)
            {
                mw.TxtBox3.Foreground = new SolidColorBrush(Colors.Red);
            }
        }
        if (now > max && now !=0)
        {
            if(i == 1)
            {
                mw.TxtBox1.Foreground = new SolidColorBrush(Colors.Red);
            }
            if (i == 2)
            {
                mw.TxtBox2.Foreground = new SolidColorBrush(Colors.Red);
            }
            if (i == 3)
            {
                mw.TxtBox3.Foreground = new SolidColorBrush(Colors.Red);
            }
        }

}

I want to do the following:

i can be between 1 and 33.

I can get the textbox name (TxtBox1) etc.

I would like to reduce the if statements so I don't have 32 if statements for each larger if statement.

Thank you! :)

if statements are not loops. They simply execute depending on the condition presented. — Glorin Oakenfoot
– Glorin Oakenfoot, Commented Nov 25, 2015 at 22:00
Cleaning up this code is a great idea! You have magic numbers, violate the DRY principle ... Suggestion: put all textboxes into an array and then access the array with index i. — Thomas Weller
– Thomas Weller, Commented Nov 25, 2015 at 22:03
The question is, can I condense the if statements down? @GlorinOakenfoot I have a for loop that calls this method, each time giving incrementing i by one 32 times. It does the for loop every 2 seconds on a timer. — Tom Pitts
– Tom Pitts, Commented Nov 25, 2015 at 22:03

Ed Bayiates · Accepted Answer · 2015-11-25 22:11:05Z

3

Just use an array for the textboxes, something like this:

internal static void isGreaterThanOrBelowValue(int min, int max, int now, int i)
{
    MainWindow mw = new MainWindow();
    TextBox[] tbList = new TextBox[] { mw.TxtBox1, mw.TxtBox2, mw.TxtBox3 };

    if (now !=0 && (now < min || now > max))
    {
        tbList[i-1].Foreground = new SolidColorBrush(Colors.Red);
    }
}

You could also build the array outside the function as a class member if the textboxes don't change, so that you were not rebuilding it every time the function is called:

static MainWindow mw = new MainWindow();
static TextBox[] tbList = new TextBox[] { mw.TxtBox1, mw.TxtBox2, mw.TxtBox3 };

internal static void isGreaterThanOrBelowValue(int min, int max, int now, int i)
{
    if (now !=0 && (now < min || now > max))
    {
        tbList[i-1].Foreground = new SolidColorBrush(Colors.Red);
    }
}

edited Nov 25, 2015 at 22:11

answered Nov 25, 2015 at 22:05

Ed Bayiates

11.3k4 gold badges47 silver badges65 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Draco18s no longer trusts SE · Accepted Answer · 2015-11-25 22:05:41Z

0

Well, for one you can do this:

if ((now < min || now > max) && now !=0)

Which simplifies your statements dramatically, as both now<min and now>max do the same thing.

For the 33 text boxes, you'd have to either put them into an array and use an index lookup, write a function called getTextBoxFromIndex() and do the giant-if-block once, or see if you can do something like mw["TxtBox"+i] (some environments will support it, others wont).

answered Nov 25, 2015 at 22:05

Draco18s no longer trusts SE

16k3 gold badges41 silver badges60 bronze badges

Comments

Seth Kitchen · Accepted Answer · 2015-11-25 22:07:38Z

0

For one you can reduce the code by using "else if" instead of "if" everytime.

When you do

if(I==1)
{
}
if(I==2)
{
}

It will check both conditions always.

When you do

if(I==1)
{
}
else if(I==2)
{
}

The second statement will only be checked if the first statement is false. Since we know i=1, we know i can't be 2, so we shouldn't check statement 2.

Also read the comments because they give good information. You can write a function that checks if i>0 and i<3 and sets the brush to red accordingly. Honestly this question is too broad. There are many ways to reduce your code.

answered Nov 25, 2015 at 22:07

Seth Kitchen

1,5661 gold badge21 silver badges55 bronze badges

Collectives™ on Stack Overflow

C# If loop within a for loop within an if loop

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related