views:

377

answers:

7

I built a little program that calculates the average of 15 numbers or less. There are 15 text-boxes, each one's default value is '0'. The program knows to get the sum of all typed numbers and divide it by numbers of text boxes that don't return '0'. But if the user deletes in mistake one of the '0'os in one of the text boxs.. run-time error.

Originally I solved this problam by writing this "if statement" 15 times(one for each text-box):

 if (t1.Text == "") { tr1 = 0; }
 else
 {
     tr1 = Double.Parse(t1.Text);
 }

this code checks if there isn't a thing in text box(for example, named t1), if true, the program is giving the double 'tr1'(don't confuse with 't1'), the value of '0', if false, the code gives the double 'tr1' the text of 't1'.

i had to write this 'if' 15 times. i wanted to know if i can write the same code with arrays and a for loop, and how?

here is the whole code (sorry for var names are not similar to var's use.):

 private void goyouidiot_Click(object sender, EventArgs e)
 {
     double tr1;
     double tr2;
     double tr3;
     double tr4;
     double tr5;
     double tr6;
     double tr7;
     double tr8;
     double tr9;
     double tr10;
     double tr11;
     double tr12;
     double tr13;
     double tr14;
     double tr15;
     if (t1.Text == "") { tr1 = 0; }
     else
     {
         tr1 = Double.Parse(t1.Text);
     }
     if (t2.Text == "") { tr2 = 0; }
     else
     {
         tr2 = Double.Parse(t2.Text);
     }

     if (t3.Text == "") { tr3 = 0; }
     else
     {
         tr3 = Double.Parse(t3.Text);
     }


     if (t4.Text == "") { tr4 = 0; }
     else
     {
         tr4 = Double.Parse(t4.Text);
     }


     if (t5.Text == "") { tr5 = 0; }
     else
     {
         tr5 = Double.Parse(t5.Text);
     }

     if (t6.Text == "") { tr6 = 0; }
     else
     {
         tr6 = Double.Parse(t6.Text);
     }


     if (t7.Text == "") { tr7 = 0; }
     else
     {
         tr7 = Double.Parse(t7.Text);
     }


     if (t8.Text == "") { tr8 = 0; }
     else
     {
         tr8 = Double.Parse(t8.Text);
     }

     if (t9.Text == "") { tr9 = 0; }
     else
     {
         tr9 = Double.Parse(t9.Text);
     }


     if (t10.Text == "") { tr10 = 0; }
     else
     {
         tr10 = Double.Parse(t10.Text);
     }


     if (t11.Text == "") { tr11 = 0; }
     else
     {
         tr11 = Double.Parse(t11.Text);
     }


     if (t12.Text == "") { tr12 = 0; }
     else
     {
         tr12 = Double.Parse(t12.Text);
     }

     if (t13.Text == "") { tr13 = 0; }
     else
     {
         tr13 = Double.Parse(t13.Text);
     }


     if (t14.Text == "") { tr14 = 0; }
     else
     {
         tr14 = Double.Parse(t14.Text);
     }


     if (t15.Text == "") { tr15 = 0; }
     else
     {
         tr15 = Double.Parse(t15.Text);
     }
     double[] sch = { tr1, tr2, tr3, tr4, tr5, tr6, tr7, tr8, tr9, tr10, tr11, tr12, tr13, tr14, tr15 };
     double total = 0;
     double sorf = 0;
     for (int i = 0; i != 14; i++)
     {

         sorf = sorf + sch[i];

         if (sch[i] > 0)
         { total++; }

     }

     double totalic = sorf / total;
     string glass = totalic.ToString();
     result.Text = ("your score: " + glass);
}
A: 

Put a .Trim() when retrieving the values from the TextBox

tr3 = Double.Parse(t3.Text.Trim());

TheGeekYouNeed
how can i use it to solve my problem? what .trim() does anyway?there is a run-time error like i tried to turn null into 0
Gilad Naaman
+7  A: 

Sure, make a double tr[15] and a corresponding array of text fields.

Then just use:

for (int i = 0; i < 15; i++) {
    if (t[i].Text == "") { 
        tr[i] = 0;
    } else {
        tr[i] = Double.Parse(t[i].Text);
    }
}
Borealid
+3  A: 

If it's just the large amount of source code taken up with your if statements, you could opt for something like:

tr1  = ( t1.Text == "") ? 0 : Double.Parse( t1.Text);
tr2  = ( t2.Text == "") ? 0 : Double.Parse( t2.Text);
:
tr15 = (t15.Text == "") ? 0 : Double.Parse(t15.Text);

This is nice and neat, doesn't take up a lot of screen real-estate and it's fairly easy to see the intent.

Or, better yet, something like:

tr1  = 0; try { tr1  = Double.Parse( t1.Text); } catch (Exception e) {};
tr2  = 0; try { tr2  = Double.Parse( t2.Text); } catch (Exception e) {};
:
tr15 = 0; try { tr15 = Double.Parse(t15.Text); } catch (Exception e) {};

because the fields could be invalid and non-blank.

You can do the same thing with arrays and a for loop if you structure your data and controls differently but it may not be necessary for only fifteen items. Certainly if you were to add more, I'd give serious consideration to that option.

And you may want to load the values directly into an array so that you don't need sch:

double tr[15];
:
tr[ 0] = 0; try { tr[ 0] = Double.Parse( t1.Text); } catch (Exception e) {};
tr[ 1] = 0; try { tr[ 1] = Double.Parse( t2.Text); } catch (Exception e) {};
:
tr[14] = 0; try { tr[14] = Double.Parse(t15.Text); } catch (Exception e) {};
:
double total = 0;
double sorf = 0;
for (int i = 0; i < 15; i++) {
    if (tr[i] > 0) {
        sorf = sorf + tr[i];
        total++;
    }
}
:

For a minimal code solution, you can also create an array of the text boxes that you're pulling the information from. Something like (untested):

TextBox t[] = {t1, t2, t3, ..., t15};
double tr[t.length];
:
for (int i = 0; i < t.length; i++) {
    tr[i] = 0; try { tr[i] = Double.Parse(t[i].Text); } catch (Exception e) {};
}
:
double total = 0;
double sorf = 0;
for (int i = 0; i < tr.length; i++) {
    if (tr[i] > 0) {
        sorf = sorf + tr[i];
        total++;
    }
}
:
paxdiablo
You have 90k rep and it never occurs to you to make a `ParseDoubleOrZero` function?
Gabe
I have a 90K rep but I'm a relative newcomer to C# :-) But your point is taken though I wonder how much of an advantage it has. Certainly none in terms of source code size. Probably less object code and more readable since you could use a decent function name.
paxdiablo
And I would use double.TryParse instead because exceptions will arise regularly if it is a user entering a number.
lasseespeholt
Thank you! Works great!
Gilad Naaman
+9  A: 
Double.TryParse(t1.Text.Trim(), out tr1);

will set tr1 to the text box's numeric value, or 0.0 if it failed to convert it for some reason. It'll also return true if the conversion succeeded or false if it failed, but you don't care about the return value if the default is 0.0.

Added bonus: it won't throw an exception if someone decides to put "This is not a number." into a text box. It'll just see the value as 0.

To do this in an array...

TextBox t[] = { t1, t2, t3, t4, t5, t6, t7, t8, t9, t10, t11, t12, t13, t14, t15 };
double tr[] = new double[t.Length];

for (int i = 0; i < t.Length; ++i)
{
    Double.TryParse(t[i].Text.Trim(), out tr[i]);
}

UPDATE:

Note, it's perfectly reasonable to expect to be able to compute an average of numbers that includes 0. In order to do this:

TextBox t[] = { t1, t2, t3, t4, t5, t6, t7, t8, t9, t10, t11, t12, t13, t14, t15 };
double tr[] = new double[t.Length];
int valid_count = 0;

for (int i = 0; i < t.Length; ++i)
{
    if (Double.TryParse(t[i].Text.Trim(), out tr[i])) ++valid_count;
}

Set your TextBoxes' default values to blank (""), and then you'll know how many were legitimately 0's entered by the user and how many were blank. Divide the sum by valid_count to get an accurate average. (But be sure valid_count > 0, or you'll likely get a divide-by-zero exception.)

cHao
+2  A: 

Write a function that converts a textbox value to a double, something like:

private static double ConvertTextboxValueToDouble(string value)
{
    double result;
    Double.TryParse(value, out result);

    return result;
}

Then create an array from your textboxes, converting their values to doubles:

double[] values = 
    {
        ConvertTextboxValueToDouble(t1.text),
        ConvertTextboxValueToDouble(t2.text),
        ConvertTextboxValueToDouble(t3.text),
...
        ConvertTextboxValueToDouble(t15.text)
    }
kristian
+1  A: 

Have you considered of using a NumericUpDown instead of an TextBox?

Also instead of writing something fifteen times you should really refactor your code and try one of the following ways:

  • Register for all your entry boxes an event, where all using the same code

    public void ValueChanged(Object sender, EventArgs e)
    {
        var numericUpDown = sender as NumericUpDown;
        if(numericUpDown  == null)
            return;
        //ToDo: Put some check code here
    }
    
  • Use some List<T> where you put all your boxes into and iterate over it to check all settings

    var myList = new List<NumericUpDown>();
    //ToDo: Put all your boxes into it
    myList.Add(numericUpDown1);
    myList.Add(numericUpDown2);
    //or get the list from somewhere else
    myList.AddRange(this.Controls.OfType<NumericUpDown>())
    
    
    //OnButtonClick
    foreach(var numericUpDown in myList)
    {
        //ToDo: Do some checking
    }
    
Oliver
numricupdown wont help in cases of averege of the numbers 100, 97, 34.the user will just get tierd of finding the number.
Gilad Naaman
i don't understand your comment. A NumericUpDown is a TextBox that only accept numbers plus two buttons to in/decrease the value. So the user can use these buttons, but he can also directly input these values like in a TextBox.
Oliver
So I don't see why should I use NumricUpDown.
Gilad Naaman
A: 

For this situations and thinking to accomplish the job in a little portion of code, i use a dirty little trick: put the controls into a panel.

If your panel is only containing desired controls (in this case, textboxes), this will be enough to store the values in a list of doubles:

    private void button1_Click(object sender, EventArgs e)
    {
        List<double> doubleList = new List<double>();

        foreach (TextBox t in panel1.Controls)
            doubleList.Add(this.checkTextBox(t));
    }

    private double checkTextBox(TextBox t)
    {
        return (t.Text != string.Empty) ? Double.Parse(t.Text.Trim()) : 0;
    }

If you can't have a panel only for textboxes and the design force you to mix controls into, you will have to do an extra check/conversion:

    private void button1_Click(object sender, EventArgs e)
    {
        List<double> doubleList = new List<double>();

        foreach (Control t in panel1.Controls)
            if(t is TextBox)
                doubleList.Add(this.checkTextBox((TextBox)t));
    }

    private double checkTextBox(TextBox t)
    {
        return (t.Text != string.Empty) ? Double.Parse(t.Text.Trim()) : 0;
    }

Greetings!

sh4