How can i make this code better than this

Question

3.00/5 (2 votes)

See more:

      private void GoButt_Click(object sender, RoutedEventArgs e)
        {
            try
            {

                double VL = Convert.ToDouble(LO.Text);

                double VA = Convert.ToDouble(AO.Text);

                double VTS = Convert.ToDouble(VL * VA);

                double GypScrew0 = Convert.ToDouble();
              

                TSF.Text = VTS.ToString();
                TMS.Text = (VL * 12 / 16).ToString();
                TMT.Text = (VL * 2 / 10).ToString();
                TMSc.Text = (VL * 12 / 16 * 4).ToString();
                

             

                if (GSL.SelectedIndex.Equals(0) & CSL.SelectedIndex.Equals(0))

                    TGB.Text = (VTS / 32).ToString();
                   TGBS.Text = (VTS * 1.5 / 258).ToString();
                 
                else if (GSL.SelectedIndex.Equals(0) & CSL.SelectedIndex.Equals(1))
                    TGB.Text = (VTS / 32 * 2).ToString();

                else if (GSL.SelectedIndex.Equals(0) & CSL.SelectedIndex.Equals(2))
                    TGB.Text = (VTS / 32 * 3).ToString();

                else if (GSL.SelectedIndex.Equals(0) & CSL.SelectedIndex.Equals(3))
                    TGB.Text = (VTS / 32 * 4).ToString();


                else if (GSL.SelectedIndex.Equals(1) & CSL.SelectedIndex.Equals(0))
                    TGB.Text = (VTS / 36).ToString();

                else if (GSL.SelectedIndex.Equals(1) & CSL.SelectedIndex.Equals(1))
                    TGB.Text = (VTS / 36 * 2).ToString();

                else if (GSL.SelectedIndex.Equals(1) & CSL.SelectedIndex.Equals(2))
                    TGB.Text = (VTS / 36 * 3).ToString();

                else if (GSL.SelectedIndex.Equals(1) & CSL.SelectedIndex.Equals(3))
                    TGB.Text = (VTS / 36 * 4).ToString();


                else if (GSL.SelectedIndex.Equals(2) & CSL.SelectedIndex.Equals(0))
                    TGB.Text = (VTS / 40).ToString();

                else if (GSL.SelectedIndex.Equals(2) & CSL.SelectedIndex.Equals(1))
                    TGB.Text = (VTS / 40 * 2).ToString();

                else if (GSL.SelectedIndex.Equals(2) & CSL.SelectedIndex.Equals(2))
                    TGB.Text = (VTS / 40 * 3).ToString();

                else if (GSL.SelectedIndex.Equals(2) & CSL.SelectedIndex.Equals(3))
                    TGB.Text = (VTS / 40 * 4).ToString();



                else if (GSL.SelectedIndex.Equals(3)& CSL.SelectedIndex.Equals(0))
                    TGB.Text = (VTS / 44).ToString();

                else if (GSL.SelectedIndex.Equals(3) & CSL.SelectedIndex.Equals(1))
                    TGB.Text = (VTS / 44 * 2).ToString();

                else if (GSL.SelectedIndex.Equals(3) & CSL.SelectedIndex.Equals(2))
                    TGB.Text = (VTS / 44 * 3).ToString();

                else if (GSL.SelectedIndex.Equals(3) & CSL.SelectedIndex.Equals(3))
                    TGB.Text = (VTS / 44 * 4).ToString();
              


            {
            }
        }
catch(Exception)
            {
           
    }
    }

Posted 3-Mar-11 1:08am

Lukann

Updated 3-Mar-11 2:18am

v2

Add a Solution

3 solutions

Solution 2

Hi Lukann,

here is optimized code.

C#

private void GoButt_Click(object sender, RoutedEventArgs e)
{
   try
   {
       double VL = Convert.ToDouble(LO.Text);
       double VA = Convert.ToDouble(AO.Text);
       double VTS = Convert.ToDouble(VL * VA);
       double GypScrew0 = Convert.ToDouble();

       TSF.Text = VTS.ToString();
       TMS.Text = (VL * 12 / 16).ToString();
       TMT.Text = (VL * 2 / 10).ToString();
       TMSc.Text = (VL * 12 / 16 * 4).ToString();

       /*Optimization start*/
       int GSL = 4 * GSL.SelectedIndex;
       int CSL = CSL.SelectedIndex + 1;
       TGB.Text = Convert.ToString(VTS / ( (32 + GSL) * CSL));            
       /*Optimization end*/ 
    }
catch(Exception)
   {
   }
}

Thanks,
Imdadhusen.

Posted 3-Mar-11 1:29am

Sunasara Imdadhusen

Comments

Manfred Rudolf Bihy 3-Mar-11 8:24am

Good answer! 5+

Lukann 3-Mar-11 8:31am

Thank you for the response it is very much appreciated.

Sergey Alexandrovich Kryukov 3-Mar-11 11:25am

Good, my 5.
And OP's variant violated D.R.Y. principle.
--SA

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Manfred Rudolf Bihy · Accepted Answer · 2011-03-03T01:25:00

You can achieve it like this:

C#

private void GoButt_Click(object sender, RoutedEventArgs e)
{
    try
    {

        double VL = Convert.ToDouble(LO.Text);

        double VA = Convert.ToDouble(AO.Text);

        double VTS = Convert.ToDouble(VL * VA);

        double GypScrew0 = Convert.ToDouble();


        TSF.Text = VTS.ToString();
        TMS.Text = (VL * 12 / 16).ToString();
        TMT.Text = (VL * 2 / 10).ToString();
        TMSc.Text = (VL * 12 / 16 * 4).ToString();

        int idxGSL = GSL.SelectedIndex;
        int idxCSL = CSL.SelectedIndex;

        double tmp = (((idxCSL + 1) * VTS) / (32 + 4 * idxGSL));
        TGB.Text = String.Format("{0}",tmp);

    }
    catch(Exception)
    {
        //Do something meaningful here
    }
}

That's all that is really needed.

Cheers!

#realJSOP · Accepted Answer · 2011-03-03T01:39:00

Try something like this (this is untested code, so you may need to tweak it a bit):

C#

//--------------------------------------------------------------------------------------------------
private void GoButt_Click(object sender, RoutedEventArgs e)
{
    double VL;
    double VA;
    double VTS;
    double GypScrew0;

    if (double.TryParse(LO.Text, out VL) &&
        double.TryParse(AO.Text, out VA) &&
        GSL.SelectedIndex >= 0 && 
        CSL.SelectedIndex >= 0)
    {
        VTS       = VL * VA;

        TSF.Text  = VTS.ToString();
        TMS.Text  = (VL * 12 / 16).ToString();
        TMT.Text  = (VL * 2 / 10).ToString();
        TMSc.Text = (VL * 12 / 16 * 4).ToString();
        
        TGB.Text  = CalcTGB(VTS, GSL.SelectedIndex, CSL.SelectedIndex).ToString();
    }
    else
    {
        throw new Exception("Something bad happened on the way to the Bijou Theater");
    }
}

//--------------------------------------------------------------------------------------------------
private double CalcTGB(double vts, int gslIndex, int cslIndex)
{
    double result = 0d;
#if DEBUG
    int divider    = 32 + (4 * cslIndex);
    int multiplier = cslIndex + 1;
    result         = vts / (double)((divider * multiplier));
#else
    result         = vts / (double)((32 + (4 * cslIndex)) * (cslIndex + 1));
#endif
    return result;
}

EDIT ===============

My method allows you to step through it in the debugger to make sure the multiplier and divider are what you expect them to be.

Final note - you shoud realy use double.TryParse (instead of Convert.ToDouble) when you're converting user-specified data to numeric values. This will eliminate the need for the try/catch block.

EDIT ===============

I modified my example to use TryParse instead of Convert.ToDouble.

How can i make this code better than this

3 solutions

Solution 1

Solution 2

Solution 3

Add your solution here

Preview 0